Add error to login input #14627

luacmartins · 2023-01-27T17:57:43Z

Details

Fixes how we display errors on the login form to align with other forms within the App, i.e.:

Separates input specific errors from server errors
Adds red bottom border to inputs with form validation error (server error still results in a green border)

Fixed Issues

$ #14622

Tests

Pre-requisite: an account with 2FA enabled

Email

Navigate to the login page
Tap on continue without entering any value for email or phone number
Verify that you see the Please enter an email or phone number error and the input border is red
Enter a random text, i.e. a
Verify that the error is cleared
Tap Continue
Verify that you see the The email entered is invalid. Please fix the format and try again. error and the input border is red
Enter a fake phone number, i.e. 1234
Verify that the error is cleared
Tap Continue
Verify that you see the I couldn't validate the phone number, please try again with the country code (e.g. +15005550006). error and that the input border is GREEN (this is a server error)
Enter your account email and tap Continue

Password

Verify that you see the password input
Tap Sign in
Verify that you see the Please enter your password error and the input border is red
Enter a random text, i.e. a
Verify that the error is cleared
Tap Sign in
Verify that you see the Incorrect password. Please try again. error and the input border is red
Enter a password that satisfies the requirements, i.e. Password1
Tap Sign in
Verify that you see the Incorrect login or password. Please try again or reset your password. error and that the input border is GREEN (this is a server error)
Enter your password
Tap Sign in

2FA

Verify that you see the 2FA input
Tap Sign in
Verify that you see the Please enter your two factor code error and the input border is red
Enter a random text, i.e. 1
Verify that the error is cleared
Tap Sign in
Verify that you see the Incorrect two factor authentication code. Please try again. error and the input border is red
Enter a 2FA code that satisfies the requirements, i.e. 123456
Verify that the form is automatically submitted
Verify that you see the Incorrect Two Factor Authentication code. Please try again. error and that the input border is GREEN (this is a server error)
Enter the correct 2FA code
Tap Sign in
Verify that you can sign in

Verify that no errors appear in the JS console

Offline tests

Email

Turn off your internet connection
Verify that you see the You appear to be offline. message below the button
Verify that the Continue button is disabled
Turn on your internet connection and enter your email
Tap Continue

Password & 2FA

Turn off your internet connection
Verify that you see the You appear to be offline. message below the button
Verify that the Sign in button is disabled

QA Steps

Same as test and offline steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

web.mov

Mobile Web - Chrome

chrome.mov

Mobile Web - Safari

safari.mov

Desktop

desktop.mov

iOS

ios.mov

Android

android.mov

melvin-bot · 2023-01-31T22:39:43Z

@aimane-chnaif @youssef-lr One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

youssef-lr · 2023-01-31T23:51:41Z

server error still results in a green border

@luacmartins any reason why only form validation errors should show a red border?

luacmartins · 2023-01-31T23:59:36Z

The API doesn't return the specific input the error is related to, so right now server errors are shown as a form level error instead of individual input. They are also not translated like other input errors. This is consistent with other forms in the App, e.g. VBA forms.

youssef-lr · 2023-02-01T00:16:27Z

Haha I see. I guess we can live with just a couple of forms that aren't using Form.

youssef-lr

LGTM and tested well!

aimane-chnaif · 2023-02-02T04:19:42Z

reviewing now

aimane-chnaif · 2023-02-02T06:08:38Z

I agree LoginForm and PasswordForm won't be converted to Form for now but just imitated.

The inconsistent behavior is that:

LoginForm and PasswordForm clears any errors (either form or server) upon text change no matter changed value is valid or not.
Form doesn't clear formError upon text change if changed value is invalid.
Form doesn't clear serverError upon text change no matter changed value is valid or not.

@luacmartins Please correct me if I am wrong. If correct, are we fine with this for now?

luacmartins · 2023-02-02T16:57:10Z

@aimane-chnaif I think those make sense for the login forms and is still the same behavior we have on staging/prod.

aimane-chnaif · 2023-02-02T17:03:50Z

Maybe Magic code should be added in test case as well since passwordless flow is now live in beta?

luacmartins · 2023-02-02T17:06:35Z

Sure, let me look into that

luacmartins · 2023-02-02T20:18:43Z

Updated!

aimane-chnaif · 2023-02-03T02:28:19Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

(magic code)

web.mov

Mobile Web - Chrome

mchrome.mov

Mobile Web - Safari

msafari.mp4

Desktop

desktop.mov

iOS

(no 2fa)

ios.mp4

Android

android.mov

luacmartins · 2023-02-06T12:43:17Z

@youssef-lr could you please re-review/approve this?

OSBotify · 2023-02-06T13:23:09Z

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

github-actions · 2023-02-06T13:51:25Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	735.439 ms → 737.936 ms (+2.497 ms, ±0.0%)
App start regularAppStart	0.016 ms → 0.016 ms (+0.001 ms, +5.1%)
App start nativeLaunch	21.000 ms → 19.767 ms (-1.233 ms, -5.9%)
App start runJsBundle	204.844 ms → 200.688 ms (-4.156 ms, -2.0%)
Open Search Page TTI	625.677 ms → 620.790 ms (-4.887 ms, -0.8%)

Show details

Name	Duration
App start TTI	Baseline Mean: 735.439 ms Stdev: 35.944 ms (4.9%) Runs: 678.7047069999389 684.5361029999331 686.1242929999717 693.7912769999821 695.3073189998977 699.0080550001003 701.0991150001064 709.6791149999481 711.6603190000169 715.2046099998988 716.3863309998997 718.7085249999072 719.2675870000385 719.6416339999996 722.6454690000974 727.9598769999575 728.4811760000885 729.2209020000882 734.7967240000144 742.6441299999133 748.6664040000178 749.201238000067 755.2622380000539 762.5963610000908 764.0725950000342 767.7775340001099 769.3907810000237 772.782494999934 788.1342660000082 805.1684469999745 807.5311010000296 808.6082679999527 Current Mean: 737.936 ms Stdev: 29.549 ms (4.0%) Runs: 683.1589410000015 684.7483250000514 688.6931280000135 690.6178540000692 700.5675419999752 712.5480979999993 714.2603990000207 714.8518159999512 722.6995109999552 724.4290400000755 725.6366369999014 727.0983760000672 728.7375819999725 730.630839999998 731.51912000007 735.694618999958 740.4098249999806 740.78539799992 741.41037399997 750.8130890000612 754.4272880000062 754.7264580000192 755.4043280000333 759.59724500007 763.9128519999795 768.1880179999862 769.5769919999875 770.3349850000814 771.2054709999356 772.5592879999895 787.081506999908 797.6351729999296
App start regularAppStart	Baseline Mean: 0.016 ms Stdev: 0.001 ms (5.9%) Runs: 0.013590000104159117 0.014201000100001693 0.014241999946534634 0.014405000023543835 0.014485999941825867 0.014567000092938542 0.014892000006511807 0.015015000011771917 0.015055999858304858 0.015096000162884593 0.015177999855950475 0.015217999927699566 0.01521800016053021 0.01525900000706315 0.015340000158175826 0.015381000004708767 0.015421000076457858 0.015544000081717968 0.015664999838918447 0.015706999925896525 0.015746999997645617 0.015909000067040324 0.016153000062331557 0.01619499991647899 0.016276000067591667 0.01631600013934076 0.016642000060528517 0.016764000058174133 0.016966999974101782 0.01700900006107986 0.017619000049307942 Current Mean: 0.016 ms Stdev: 0.001 ms (7.9%) Runs: 0.013996999943628907 0.014771000016480684 0.014932999853044748 0.015054999850690365 0.015177000081166625 0.015381000004708767 0.015421000076457858 0.015462999930605292 0.015503000002354383 0.015664999838918447 0.01566500007174909 0.015705999918282032 0.015949999913573265 0.015949999913573265 0.016032000072300434 0.016072999918833375 0.016112999990582466 0.016275999834761024 0.016276000067591667 0.016276000067591667 0.016357999993488193 0.016561000142246485 0.016885999822989106 0.017333999974653125 0.01745599997229874 0.017659999895840883 0.017740000039339066 0.018555000191554427 0.019490999868139625 0.019530999939888716
App start nativeLaunch	Baseline Mean: 21.000 ms Stdev: 2.664 ms (12.7%) Runs: 18 18 18 18 18 19 19 19 19 19 19 19 20 20 20 20 21 21 21 21 21 22 22 22 23 24 25 26 26 26 27 Current Mean: 19.767 ms Stdev: 1.667 ms (8.4%) Runs: 17 18 18 18 18 18 18 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 21 21 22 22 24 24
App start runJsBundle	Baseline Mean: 204.844 ms Stdev: 22.341 ms (10.9%) Runs: 157 169 179 183 183 183 184 185 190 190 196 197 198 199 201 203 205 207 208 210 213 213 213 215 222 225 226 228 228 233 246 266 Current Mean: 200.688 ms Stdev: 16.764 ms (8.4%) Runs: 166 172 172 177 187 187 188 190 192 192 193 194 196 196 197 198 198 201 201 201 208 210 210 212 214 214 220 225 227 227 228 229
Open Search Page TTI	Baseline Mean: 625.677 ms Stdev: 23.935 ms (3.8%) Runs: 577.7195640001446 591.1077470001765 592.8811039999127 597.7614339999855 598.9297690000385 605.3092040000483 610.2122399997897 610.9771330000367 613.5528569999151 613.5954599999823 615.9999589999206 616.68697099993 618.1648359999526 619.4914549998939 619.494709999999 621.3610430001281 621.3791510001756 623.4537349999882 624.1166179999709 626.7807620000094 627.6733810000587 630.0254730000161 631.8901770000812 635.6033929998521 641.6273199999705 642.489625000162 657.8360190000385 658.0266519999132 658.7054439999629 661.992838999955 677.0683190000709 679.7423499999568 Current Mean: 620.790 ms Stdev: 15.766 ms (2.5%) Runs: 598.0346269998699 600.0327959998976 601.9103600000963 602.5914309998043 604.6868900000118 605.0747070000507 605.652710000053 608.1223960001953 608.3635670000222 610.9959309999831 611.2755950000137 611.5906579999719 612.4532070001587 613.1805420001037 615.3208419999573 615.6926669999957 618.5207119998522 620.7231850000098 622.3173020000104 623.5973710000981 624.1275639999658 624.522786000045 627.2038169999141 629.820963999955 633.1833099999931 636.8682049999479 639.5628659999929 642.6184489999432 643.5481370000634 646.2128500000108 647.6479080000427 659.8191329999827

OSBotify · 2023-02-06T21:46:05Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 1.2.66-0 🚀

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

OSBotify · 2023-02-07T17:58:39Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.2.66-0 🚀

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

mananjadhav · 2023-04-27T09:46:59Z

I am tagging this PR to highlight an issue fixed here. All conditions in ternary expressions or left-hand operands on conditional renders, should be boolean. This PR is one of the PRs that uses conditional render with string operands, hence I am tagging it here for the contributors to check.

We've also updated the item in the checklist with this PR to avoid this issue in the future.

luacmartins added 2 commits January 27, 2023 10:13

add error to textinput

bbbbc7c

add error for email field

56c3d7d

luacmartins self-assigned this Jan 27, 2023

luacmartins added 19 commits January 27, 2023 14:07

rm hasError

99de668

import form

62f05ef

fix layout

ad1a7a5

update submit button text

90abcf6

rm input props

39505dd

create validate function

c75b744

update submit function

0d57d90

rm errors

88b835f

rm formError state

c4f3823

revert form changes

f4e58b9

rm componentDidUpdate logic

320bb44

add consts for errors

07f0c98

fix style

b4e40ff

Merge branch 'main' into cmartins-addErrorToLoginInput

4dd52eb

fix errors in login form

d530dcc

align errors with form design

24e5cf0

create onTextInput

9f1f5e7

use FormAlertWithSubmitButton

1590323

use button

60b9d02

luacmartins marked this pull request as ready for review January 31, 2023 22:39

luacmartins requested a review from a team as a code owner January 31, 2023 22:39

melvin-bot bot requested review from aimane-chnaif and youssef-lr and removed request for a team January 31, 2023 22:39

youssef-lr previously approved these changes Feb 2, 2023

View reviewed changes

luacmartins added 2 commits February 2, 2023 12:18

Merge branch 'main' into cmartins-addErrorToLoginInput

818f998

update validatecode form

f8363ff

luacmartins dismissed youssef-lr’s stale review via f8363ff February 2, 2023 20:05

luacmartins added 2 commits February 2, 2023 13:09

add error for invalid 2fa

c533248

rm unecessary bindings

b6da1f5

aimane-chnaif approved these changes Feb 3, 2023

View reviewed changes

youssef-lr approved these changes Feb 6, 2023

View reviewed changes

luacmartins merged commit 757b0e9 into main Feb 6, 2023

luacmartins deleted the cmartins-addErrorToLoginInput branch February 6, 2023 13:17

OSBotify mentioned this pull request Feb 6, 2023

Deploy Checklist: New Expensify 2023-02-06 #14883

Closed

6 tasks

MelvinBot mentioned this pull request Feb 7, 2023

[PAID] [HOLD for payment 2023-02-14] [$1000] Login - The line is green when incorrect password entered #14622

Closed

2 tasks

grgia mentioned this pull request Apr 12, 2023

New Branding - New Sign In Imagery #15737

Merged

65 tasks

alexxxwork mentioned this pull request Apr 21, 2023

[HOLD for payment 2023-05-05] [HOLD for payment 2023-04-20] [$1000] Text strings must be rendered within a <Text> component #16683

Closed

paultsimura mentioned this pull request Dec 6, 2023

[HOLD for payment 2023-12-28] [$500] Login - Two error messages appear simultaneously #32559

Closed

6 tasks

jakub-trzebiatowski mentioned this pull request Dec 13, 2023

fix: Login – show only input error if both are present #32944

Merged

46 tasks

Add error to login input #14627

Add error to login input #14627

Uh oh!

Conversation

luacmartins commented Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot bot commented Jan 31, 2023

Uh oh!

youssef-lr commented Jan 31, 2023

Uh oh!

luacmartins commented Jan 31, 2023

Uh oh!

youssef-lr commented Feb 1, 2023

Uh oh!

youssef-lr left a comment

Choose a reason for hiding this comment

Uh oh!

aimane-chnaif commented Feb 2, 2023

Uh oh!

aimane-chnaif commented Feb 2, 2023

Uh oh!

luacmartins commented Feb 2, 2023

Uh oh!

aimane-chnaif commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luacmartins commented Feb 2, 2023

Uh oh!

luacmartins commented Feb 2, 2023

Uh oh!

aimane-chnaif commented Feb 3, 2023

Reviewer Checklist

Screenshots/Videos

Uh oh!

luacmartins commented Feb 6, 2023

Uh oh!

OSBotify commented Feb 6, 2023

Uh oh!

github-actions bot commented Feb 6, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

Uh oh!

OSBotify commented Feb 6, 2023

Uh oh!

OSBotify commented Feb 7, 2023

Uh oh!

mananjadhav commented Apr 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

luacmartins commented Jan 27, 2023 •

edited

Loading

aimane-chnaif commented Feb 2, 2023 •

edited

Loading