Add conditional passwordless flow #13655
Conversation
…ionalPasswordlessFlow
…ionalPasswordlessFlow
…ionalPasswordlessFlow
There was a problem hiding this comment.
Test well on all platforms! Although, I still can't resend the magic code, and the email I received with the invalidated account is a magic link, not the magic code.
Checklist #13655 (comment).
@mollfpr what do you mean by this? Do you have a screenshot of the notification you received? |
|
@NikkiWines Sign up with a new unvalidated account on password less will show the validation form, but the email I receive is a link to set the password.
|
|
Hmm that should definitely be a different notification 🤔 will investigate |
|
@mollfpr if you use |
|
Yes, I got the notification that it has a magic link and magic code.
Yes! I probably mixed up with the dev environment, where it’s open to all beta permission. |
|
Ok, perfect, then I think we're all good |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Performance Comparison Report 📊Significant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
|
|
🚀 Deployed to staging by https://github.com/NikkiWines in version: 1.2.64-0 🚀
|
|
I see that you can disable the passwordless login by returning |
|
@aldo-expensify yeah pretty much - we need to be able to show both flows for now since passwordless is still in beta, but in the future we'll definitely remove all that beta shtuff so the user will only have the passwordless flow option |
|
Tests in staging: On Passwordless beta
Not on Passwordless beta
Possible failure:
Another possible failure:
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀
|
1 similar comment
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀
|
| componentDidMount() { | ||
| if (!canFocusInputOnScreenFocus() || !this.inputValidateCode || !this.props.isVisible) { | ||
| return; | ||
| } | ||
| this.inputValidateCode.focus(); | ||
| } | ||
|
|
||
| componentDidUpdate(prevProps, prevState) { | ||
| if (!prevProps.isVisible && this.props.isVisible) { | ||
| this.inputValidateCode.focus(); | ||
| } |
There was a problem hiding this comment.
@Beamanator in componentDidUpdate we are not checking for canFocusInputOnScreenFocus should not we do so?
componentDidUpdate(prevProps) {
if (canFocusInputOnScreenFocus() && !prevProps.isVisible && this.props.isVisible) {
this.inputValidateCode.focus();
}
...
}
|
We decided to disable "Didn't receive a magic code?" when user is offline. It was not considered or missed in this PR. Issue: #19867 |
|
Issue: #22324 Title: "Incorrect magic code" error if click login in 2FA input page This issue was missed and not handled in this PR. |
|
|
||
| render() { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
The react fragment wrapper here caused the following issue:
Which is a Safari specific autofill popup issue that was observed by others, see this SO (comment) and #32683 (comment) breakdown for more details.
Ready for tests / review! 👍
Details
Adding the ability to show the new "Passwordless" flow if the user is in the Passwordless beta (so we can do a slow ish, smart rollout)
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/249360
Tests
To add an account to the Passwordless beta:
$allBetasEnabled = false;$toggledBetas = ['passwordless'];To make the account not be on the
passwordlessbeta:src/libs/Permissions.jsand make the functioncanUsePasswordlessLoginshavereturn false;lib/BetaManager.phpand make sure$allBetasEnabledis set tofalseWith an unvalidated account, on the Passwordless beta
000000, then click "Sign in"With a validated account, on the Passwordless beta
000000into the form, then click "Sign in"With a validated account on the Passwordless beta with 2FA required
000000into the form, then click "Sign in"With an unvalidated account, not on the Passwordless beta
php script/notifyall.phpfrom your VMWith a validated account, not on the Passwordless beta
php script/notifyall.phpfrom your VMWith a validated account not on the Passwordless beta with 2FA required
In all tests:
Offline tests
None of the above flows should work offline, as the signin flows all require being online.
Verify if you try to sign in while offline, you're unable (the signin button should be disabled)
QA Steps
Similar as above, but will need an internal employee to add the accounts to
passwordlessbeta on stagingTo add an account to the Passwordless beta:
passwordlessbetaIn all tests:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.Screenshots/Videos
Web
On Passwordless beta
Not on Passwordless beta
Mobile Web - Chrome
On Passwordless beta
Not on Passwordless beta
Mobile Web - Safari
On Passwordless beta
Not on Passwordless beta
Desktop
On Passwordless beta
Not on Passwordless beta
iOS
On Passwordless beta
Not on Passwordless beta
Android
On Passwordless beta
Not on Passwordless beta