-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
Migrate SignInPage to functional component #19498
Changes from 17 commits
829a8af
9c70f34
debfadd
d6c6f73
c2288d6
eacbc0d
5d9a914
9fb84a2
bcae04a
2cb906c
03f53a2
1231088
6b9a9c7
1ae5d86
773e231
ebc566c
6ca6ff0
2972805
3c58737
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import {useContext} from 'react'; | ||
import {LocaleContext} from '../components/withLocalize'; | ||
|
||
export default function useLocalize() { | ||
return useContext(LocaleContext); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import _ from 'underscore'; | ||
import {useContext, useMemo} from 'react'; | ||
import Permissions from '../libs/Permissions'; | ||
import {BetasContext} from '../components/OnyxProvider'; | ||
|
||
export default function usePermissions() { | ||
const betas = useContext(BetasContext); | ||
return useMemo( | ||
() => | ||
_.reduce( | ||
Permissions, | ||
(memo, checkerFunction, beta) => { | ||
// eslint-disable-next-line no-param-reassign | ||
memo[beta] = checkerFunction(betas); | ||
return memo; | ||
}, | ||
{}, | ||
), | ||
[betas], | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,27 @@ | ||
import React, {Component} from 'react'; | ||
import React, {useEffect, useMemo} from 'react'; | ||
import {View} from 'react-native'; | ||
import PropTypes from 'prop-types'; | ||
import {withOnyx} from 'react-native-onyx'; | ||
import {View} from 'react-native'; | ||
import lodashGet from 'lodash/get'; | ||
import Str from 'expensify-common/lib/str'; | ||
import {withSafeAreaInsets} from 'react-native-safe-area-context'; | ||
import {useSafeAreaInsets} from 'react-native-safe-area-context'; | ||
import ONYXKEYS from '../../ONYXKEYS'; | ||
import styles from '../../styles/styles'; | ||
import compose from '../../libs/compose'; | ||
import SignInPageLayout from './SignInPageLayout'; | ||
import LoginForm from './LoginForm'; | ||
import PasswordForm from './PasswordForm'; | ||
import ValidateCodeForm from './ValidateCodeForm'; | ||
import ResendValidationForm from './ResendValidationForm'; | ||
import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; | ||
import Performance from '../../libs/Performance'; | ||
import * as App from '../../libs/actions/App'; | ||
import Permissions from '../../libs/Permissions'; | ||
import UnlinkLoginForm from './UnlinkLoginForm'; | ||
import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/withWindowDimensions'; | ||
import * as Localize from '../../libs/Localize'; | ||
import useLocalize from '../../hooks/useLocalize'; | ||
import usePermissions from '../../hooks/usePermissions'; | ||
import useWindowDimensions from '../../hooks/useWindowDimensions'; | ||
import Log from '../../libs/Log'; | ||
import * as StyleUtils from '../../styles/StyleUtils'; | ||
|
||
const propTypes = { | ||
/* Onyx Props */ | ||
|
||
/** The details about the account that the user is signing in with */ | ||
account: PropTypes.shape({ | ||
/** Error to display when there is an account error returned */ | ||
|
@@ -35,153 +32,163 @@ const propTypes = { | |
|
||
/** The primaryLogin associated with the account */ | ||
primaryLogin: PropTypes.string, | ||
}), | ||
|
||
/** List of betas available to current user */ | ||
betas: PropTypes.arrayOf(PropTypes.string), | ||
/** Has the user pressed the forgot password button? */ | ||
forgotPassword: PropTypes.bool, | ||
|
||
/** Does this account require 2FA? */ | ||
requiresTwoFactorAuth: PropTypes.bool, | ||
}), | ||
|
||
/** The credentials of the person signing in */ | ||
credentials: PropTypes.shape({ | ||
login: PropTypes.string, | ||
password: PropTypes.string, | ||
twoFactorAuthCode: PropTypes.string, | ||
validateCode: PropTypes.string, | ||
}), | ||
|
||
...withLocalizePropTypes, | ||
|
||
...windowDimensionsPropTypes, | ||
}; | ||
|
||
const defaultProps = { | ||
account: {}, | ||
betas: [], | ||
credentials: {}, | ||
}; | ||
|
||
class SignInPage extends Component { | ||
componentDidMount() { | ||
Performance.measureTTI(); | ||
/** | ||
* @param {Boolean} hasLogin | ||
* @param {Boolean} hasPassword | ||
* @param {Boolean} hasValidateCode | ||
* @param {Boolean} isPrimaryLogin | ||
* @param {Boolean} isAccountValidated | ||
* @param {Boolean} didForgetPassword | ||
* @param {Boolean} canUsePasswordlessLogins | ||
* @returns {Object} | ||
*/ | ||
function getRenderOptions({hasLogin, hasPassword, hasValidateCode, isPrimaryLogin, isAccountValidated, didForgetPassword, canUsePasswordlessLogins}) { | ||
const shouldShowLoginForm = !hasLogin && !hasValidateCode; | ||
const isUnvalidatedSecondaryLogin = hasLogin && !isPrimaryLogin && !isAccountValidated; | ||
const shouldShowPasswordForm = hasLogin && isAccountValidated && !hasPassword && !didForgetPassword && !isUnvalidatedSecondaryLogin && !canUsePasswordlessLogins; | ||
const shouldShowValidateCodeForm = (hasLogin || hasValidateCode) && !isUnvalidatedSecondaryLogin && canUsePasswordlessLogins; | ||
const shouldShowResendValidationForm = hasLogin && (!isAccountValidated || didForgetPassword) && !isUnvalidatedSecondaryLogin && !canUsePasswordlessLogins; | ||
const shouldShowWelcomeHeader = shouldShowLoginForm || shouldShowPasswordForm || shouldShowValidateCodeForm || isUnvalidatedSecondaryLogin; | ||
const shouldShowWelcomeText = shouldShowLoginForm || shouldShowPasswordForm || shouldShowValidateCodeForm; | ||
Comment on lines
+67
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add back the comments which were present before the refactor. They were useful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree. I feel these variables have been renamed such that the comments became trivial / added no value. For some examples, we would've had this:
Each line in the comment is just saying the same thing as the boolean statement in more words. not useful imo |
||
return { | ||
shouldShowLoginForm, | ||
shouldShowUnlinkLoginForm: isUnvalidatedSecondaryLogin, | ||
shouldShowPasswordForm, | ||
shouldShowValidateCodeForm, | ||
shouldShowResendValidationForm, | ||
shouldShowWelcomeHeader, | ||
shouldShowWelcomeText, | ||
}; | ||
} | ||
|
||
function SignInPage({account, credentials}) { | ||
const {translate, formatPhoneNumber} = useLocalize(); | ||
roryabraham marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const {canUsePasswordlessLogins} = usePermissions(); | ||
const {isSmallScreenWidth} = useWindowDimensions(); | ||
const safeAreaInsets = useSafeAreaInsets(); | ||
|
||
useEffect(() => Performance.measureTTI(), []); | ||
useEffect(() => { | ||
App.setLocale(Localize.getDevicePreferredLocale()); | ||
} | ||
|
||
render() { | ||
// Show the login form if | ||
// - A login has not been entered yet | ||
// - AND a validateCode has not been cached with sign in link | ||
const showLoginForm = !this.props.credentials.login && !this.props.credentials.validateCode; | ||
|
||
// Show the unlink form if | ||
// - A login has been entered | ||
// - AND the login is not the primary login | ||
// - AND the login is not validated | ||
const showUnlinkLoginForm = | ||
Boolean(this.props.credentials.login && this.props.account.primaryLogin) && this.props.account.primaryLogin !== this.props.credentials.login && !this.props.account.validated; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here we also check for the |
||
|
||
// Show the old password form if | ||
// - A login has been entered | ||
// - AND an account exists and is validated for this login | ||
// - AND a password hasn't been entered yet | ||
// - AND haven't forgotten password | ||
// - AND the login isn't an unvalidated secondary login | ||
// - AND the user is NOT on the passwordless beta | ||
const showPasswordForm = | ||
Boolean(this.props.credentials.login) && | ||
this.props.account.validated && | ||
!this.props.credentials.password && | ||
!this.props.account.forgotPassword && | ||
!showUnlinkLoginForm && | ||
!Permissions.canUsePasswordlessLogins(this.props.betas); | ||
|
||
// Show the new magic code / validate code form if | ||
// - A login has been entered or a validateCode has been cached from sign in link | ||
// - AND the login isn't an unvalidated secondary login | ||
// - AND the user is on the 'passwordless' beta | ||
const showValidateCodeForm = | ||
Boolean(this.props.credentials.login || this.props.credentials.validateCode) && !showUnlinkLoginForm && Permissions.canUsePasswordlessLogins(this.props.betas); | ||
|
||
// Show the resend validation link form if | ||
// - A login has been entered | ||
// - AND is not validated or password is forgotten | ||
// - AND the login isn't an unvalidated secondary login | ||
// - AND user is not on 'passwordless' beta | ||
const showResendValidationForm = | ||
Boolean(this.props.credentials.login) && | ||
(!this.props.account.validated || this.props.account.forgotPassword) && | ||
!showUnlinkLoginForm && | ||
!Permissions.canUsePasswordlessLogins(this.props.betas); | ||
|
||
let welcomeHeader = ''; | ||
let welcomeText = ''; | ||
if (showValidateCodeForm) { | ||
if (this.props.account.requiresTwoFactorAuth) { | ||
}, []); | ||
|
||
const { | ||
shouldShowLoginForm, | ||
shouldShowUnlinkLoginForm, | ||
shouldShowPasswordForm, | ||
shouldShowValidateCodeForm, | ||
shouldShowResendValidationForm, | ||
shouldShowWelcomeHeader, | ||
shouldShowWelcomeText, | ||
} = getRenderOptions({ | ||
hasLogin: Boolean(credentials.login), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but here we dont have it, I have tried changing this to |
||
hasPassword: Boolean(credentials.password), | ||
hasValidateCode: Boolean(credentials.validateCode), | ||
isPrimaryLogin: account.primaryLogin && account.primaryLogin === credentials.login, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion
|
||
isAccountValidated: Boolean(account.validated), | ||
didForgetPassword: Boolean(account.forgotPassword), | ||
canUsePasswordlessLogins, | ||
}); | ||
|
||
const {welcomeHeader, welcomeText} = useMemo(() => { | ||
roryabraham marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let header = ''; | ||
let text = ''; | ||
if (shouldShowValidateCodeForm) { | ||
if (account.requiresTwoFactorAuth) { | ||
// We will only know this after a user signs in successfully, without their 2FA code | ||
welcomeHeader = this.props.isSmallScreenWidth ? '' : this.props.translate('welcomeText.welcomeBack'); | ||
welcomeText = this.props.translate('validateCodeForm.enterAuthenticatorCode'); | ||
header = isSmallScreenWidth ? '' : translate('welcomeText.welcomeBack'); | ||
text = translate('validateCodeForm.enterAuthenticatorCode'); | ||
} else { | ||
const userLogin = Str.removeSMSDomain(lodashGet(this.props, 'credentials.login', '')); | ||
const userLogin = Str.removeSMSDomain(credentials.login || ''); | ||
|
||
// replacing spaces with "hard spaces" to prevent breaking the number | ||
const userLoginToDisplay = Str.isSMSLogin(userLogin) ? this.props.formatPhoneNumber(userLogin).replace(/ /g, '\u00A0') : userLogin; | ||
if (this.props.account.validated) { | ||
welcomeHeader = this.props.isSmallScreenWidth ? '' : this.props.translate('welcomeText.welcomeBack'); | ||
welcomeText = this.props.isSmallScreenWidth | ||
? `${this.props.translate('welcomeText.welcomeBack')} ${this.props.translate('welcomeText.welcomeEnterMagicCode', {login: userLoginToDisplay})}` | ||
: this.props.translate('welcomeText.welcomeEnterMagicCode', {login: userLoginToDisplay}); | ||
const userLoginToDisplay = Str.isSMSLogin(userLogin) ? formatPhoneNumber(userLogin).replace(/ /g, '\u00A0') : userLogin; | ||
if (account.validated) { | ||
header = isSmallScreenWidth ? '' : translate('welcomeText.welcomeBack'); | ||
text = isSmallScreenWidth | ||
? `${translate('welcomeText.welcomeBack')} ${translate('welcomeText.welcomeEnterMagicCode', {login: userLoginToDisplay})}` | ||
: translate('welcomeText.welcomeEnterMagicCode', {login: userLoginToDisplay}); | ||
} else { | ||
welcomeHeader = this.props.isSmallScreenWidth ? '' : this.props.translate('welcomeText.welcome'); | ||
welcomeText = this.props.isSmallScreenWidth | ||
? `${this.props.translate('welcomeText.welcome')} ${this.props.translate('welcomeText.newFaceEnterMagicCode', {login: userLoginToDisplay})}` | ||
: this.props.translate('welcomeText.newFaceEnterMagicCode', {login: userLoginToDisplay}); | ||
header = isSmallScreenWidth ? '' : translate('welcomeText.welcome'); | ||
text = isSmallScreenWidth | ||
? `${translate('welcomeText.welcome')} ${translate('welcomeText.newFaceEnterMagicCode', {login: userLoginToDisplay})}` | ||
: translate('welcomeText.newFaceEnterMagicCode', {login: userLoginToDisplay}); | ||
} | ||
} | ||
} else if (showPasswordForm) { | ||
welcomeHeader = this.props.isSmallScreenWidth ? '' : this.props.translate('welcomeText.welcomeBack'); | ||
welcomeText = this.props.isSmallScreenWidth | ||
? `${this.props.translate('welcomeText.welcomeBack')} ${this.props.translate('welcomeText.enterPassword')}` | ||
: this.props.translate('welcomeText.enterPassword'); | ||
} else if (showUnlinkLoginForm) { | ||
welcomeHeader = this.props.isSmallScreenWidth ? this.props.translate('login.hero.header') : this.props.translate('welcomeText.welcomeBack'); | ||
} else if (!showResendValidationForm) { | ||
welcomeHeader = this.props.isSmallScreenWidth ? this.props.translate('login.hero.header') : this.props.translate('welcomeText.getStarted'); | ||
welcomeText = this.props.isSmallScreenWidth ? this.props.translate('welcomeText.getStarted') : ''; | ||
} else if (shouldShowPasswordForm) { | ||
header = isSmallScreenWidth ? '' : translate('welcomeText.welcomeBack'); | ||
text = isSmallScreenWidth ? `${translate('welcomeText.welcomeBack')} ${translate('welcomeText.enterPassword')}` : translate('welcomeText.enterPassword'); | ||
} else if (shouldShowUnlinkLoginForm) { | ||
header = isSmallScreenWidth ? translate('login.hero.header') : translate('welcomeText.welcomeBack'); | ||
} else if (!shouldShowResendValidationForm) { | ||
header = isSmallScreenWidth ? translate('login.hero.header') : translate('welcomeText.getStarted'); | ||
text = isSmallScreenWidth ? translate('welcomeText.getStarted') : ''; | ||
} else { | ||
Log.warn('SignInPage in unexpected state!'); | ||
} | ||
|
||
return ( | ||
// There is an issue SafeAreaView on Android where wrong insets flicker on app start. | ||
// Can be removed once https://github.com/th3rdwave/react-native-safe-area-context/issues/364 is resolved. | ||
<View style={[styles.signInPage, StyleUtils.getSafeAreaPadding(this.props.insets, 1)]}> | ||
<SignInPageLayout | ||
welcomeHeader={welcomeHeader} | ||
welcomeText={welcomeText} | ||
shouldShowWelcomeHeader={showLoginForm || showPasswordForm || showValidateCodeForm || showUnlinkLoginForm || !this.props.isSmallScreenWidth} | ||
shouldShowWelcomeText={showLoginForm || showPasswordForm || showValidateCodeForm} | ||
> | ||
{/* LoginForm and PasswordForm must use the isVisible prop. This keeps them mounted, but visually hidden | ||
return {welcomeHeader: header, welcomeText: text}; | ||
}, [ | ||
shouldShowValidateCodeForm, | ||
shouldShowPasswordForm, | ||
shouldShowUnlinkLoginForm, | ||
shouldShowResendValidationForm, | ||
account.requiresTwoFactorAuth, | ||
account.validated, | ||
credentials.login, | ||
isSmallScreenWidth, | ||
translate, | ||
formatPhoneNumber, | ||
]); | ||
|
||
return ( | ||
<View style={[styles.signInPage, StyleUtils.getSafeAreaPadding(safeAreaInsets, 1)]}> | ||
<SignInPageLayout | ||
welcomeHeader={welcomeHeader} | ||
welcomeText={welcomeText} | ||
shouldShowWelcomeHeader={shouldShowWelcomeHeader || !isSmallScreenWidth} | ||
shouldShowWelcomeText={shouldShowWelcomeText} | ||
> | ||
{/* LoginForm and PasswordForm must use the isVisible prop. This keeps them mounted, but visually hidden | ||
so that password managers can access the values. Conditionally rendering these components will break this feature. */} | ||
<LoginForm | ||
isVisible={showLoginForm} | ||
blurOnSubmit={this.props.account.validated === false} | ||
/> | ||
{showValidateCodeForm ? <ValidateCodeForm isVisible={showValidateCodeForm} /> : <PasswordForm isVisible={showPasswordForm} />} | ||
{showResendValidationForm && <ResendValidationForm />} | ||
{showUnlinkLoginForm && <UnlinkLoginForm />} | ||
</SignInPageLayout> | ||
</View> | ||
); | ||
} | ||
<LoginForm | ||
isVisible={shouldShowLoginForm} | ||
blurOnSubmit={account.validated === false} | ||
/> | ||
{shouldShowValidateCodeForm ? <ValidateCodeForm isVisible={shouldShowValidateCodeForm} /> : <PasswordForm isVisible={shouldShowPasswordForm} />} | ||
{shouldShowResendValidationForm && <ResendValidationForm />} | ||
{shouldShowUnlinkLoginForm && <UnlinkLoginForm />} | ||
</SignInPageLayout> | ||
</View> | ||
); | ||
} | ||
|
||
SignInPage.propTypes = propTypes; | ||
SignInPage.defaultProps = defaultProps; | ||
SignInPage.displayName = 'SignInPage'; | ||
|
||
export default compose( | ||
withSafeAreaInsets, | ||
withLocalize, | ||
withWindowDimensions, | ||
withOnyx({ | ||
account: {key: ONYXKEYS.ACCOUNT}, | ||
betas: {key: ONYXKEYS.BETAS}, | ||
credentials: {key: ONYXKEYS.CREDENTIALS}, | ||
}), | ||
)(SignInPage); | ||
export default withOnyx({ | ||
account: {key: ONYXKEYS.ACCOUNT}, | ||
credentials: {key: ONYXKEYS.CREDENTIALS}, | ||
})(SignInPage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, it will be cleaner to break this into statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but it would be a lot more verbose, and anyone adding or removing betas would also have to remember to update the hook. This way, you just have to add a new function to
libs/Permissions
and the hook will automatically handle that. This seems more robust to me.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that is fine. I was just saying to break this nested logic into multiple statements. It is hard to read.