Details

Reviewers

tomek
ashoat
atul
kamil

Commits

rCOMM81a47b30ffb0: [web][native] Trimming whitespaces from username at login screen

Summary

Solving this issue: https://linear.app/comm/issue/ENG-2041/[web]-[native]-bug-whitespaces-arent-trimmed-from-right-side-of

Added onBlur event listener to username field so whenever it goes out of focus, it is trimmed.
It goes out of focus whenever we swap fields or click submit button, so it works.

On native we simply trim whitespaces whenever user types them as blur events aren't handled in a same way (input field doesn't go out of focus when we click submit button, so there's no good place for trimming it)

Mobile:

Web:

Test Plan

Build application.
Tested scenarios manually.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• przemek created this revision.Oct 14 2022, 3:07 AM

Herald added a subscriber: • abosh. · View Herald TranscriptOct 14 2022, 3:07 AM

• przemek edited the summary of this revision. (Show Details)Oct 14 2022, 3:13 AM

• przemek edited the summary of this revision. (Show Details)

• przemek attached a referenced file: F198912: Screen Recording 2022-10-14 at 12.06.22.mov. (Show Details)

• przemek attached a referenced file: F198913: Screen Recording 2022-10-14 at 12.06.57.mov. (Show Details)

Harbormaster completed remote builds in B12791: Diff 17566.Oct 14 2022, 3:18 AM

• przemek requested review of this revision.Oct 14 2022, 3:18 AM

• przemek added 1 blocking reviewer(s): tomek.Oct 14 2022, 3:24 AM

ashoat requested changes to this revision.Oct 14 2022, 5:39 AM

ashoat added inline comments.

web/account/log-in-form.react.js
97 ↗	(On Diff #17566)	This change doesn't have anything to do with this diff, so it should be in its own diff We should make sure we're consistent across all error messages like this (eg. in registration as well) if we're making this change I'd like to see a visual comparison of lowercase / uppercase here if we want to make this change, to make sure it looks better
136 ↗	(On Diff #17566)	By defining this function inline you're causing it to be recalculated on every render. Instead, it should be defined using `React.useCallback`

This revision now requires changes to proceed.Oct 14 2022, 5:39 AM

Updating D5365: [web][native] Trimming whitespaces from username at login screen

Yeah, you're right, we should be consistent in our error messages. Sorry for that. I reversed that change.
Also added useCallback.

Harbormaster completed remote builds in B12795: Diff 17571.Oct 14 2022, 6:16 AM

Thanks for sharing a video with your test!! I was going to suggest testing if onBlur triggered before the submit if you pressed the button after having the username input focused, but I can see you already tested that :)

tomek accepted this revision.Oct 14 2022, 8:06 AM

tomek added inline comments.