fix: 2FA bug fixes #19260
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1135,4 +1135,4 @@ SPEC CHECKSUMS: | |
|
|
||
| PODFILE CHECKSUM: 4ed1c7b099741c82e2b0411b95f6468e72be6c76 | ||
|
|
||
| COCOAPODS: 1.12.0 | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,20 +117,21 @@ class ValidateLoginPage extends Component { | |
| } | ||
|
|
||
| render() { | ||
| const is2FARequired = lodashGet(this.props, 'account.requiresTwoFactorAuth', false); | ||
| const isSignedIn = Boolean(lodashGet(this.props, 'session.authToken', null)); | ||
| const currentAuthState = this.getAutoAuthState(); | ||
| return ( | ||
| <> | ||
| {currentAuthState === CONST.AUTO_AUTH_STATE.FAILED && <ExpiredValidateCodeModal />} | ||
| {currentAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && is2FARequired && !isSignedIn && <TfaRequiredModal />} | ||
| {currentAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED && isSignedIn && <AbracadabraModal />} | ||
| {currentAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED && !isSignedIn && ( | ||
|
There was a problem hiding this comment. @BeeMargarida Do you remember why we had to add the There was a problem hiding this comment. @s77rt Yes, it came from this commit to fix this bug mentioned in this slack thread. Before, it was necessary because the check for |
||
| <ValidateCodeModal | ||
| accountID={this.getAccountID()} | ||
| code={this.getValidateCode()} | ||
| /> | ||
| )} | ||
| {currentAuthState === CONST.AUTO_AUTH_STATE.SIGNING_IN && <FullScreenLoadingIndicator />} | ||
| </> | ||
| ); | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import React, {useEffect, useState} from 'react'; | ||
| import {withOnyx} from 'react-native-onyx'; | ||
| import {ActivityIndicator, View} from 'react-native'; | ||
| import {ScrollView} from 'react-native-gesture-handler'; | ||
| import _ from 'underscore'; | ||
| import PropTypes from 'prop-types'; | ||
| import HeaderWithCloseButton from '../../../../components/HeaderWithCloseButton'; | ||
|
|
@@ -11,6 +12,7 @@ import compose from '../../../../libs/compose'; | |
| import ROUTES from '../../../../ROUTES'; | ||
| import FullPageOfflineBlockingView from '../../../../components/BlockingViews/FullPageOfflineBlockingView'; | ||
| import * as Illustrations from '../../../../components/Icon/Illustrations'; | ||
| import withWindowDimensions, {windowDimensionsPropTypes} from '../../../../components/withWindowDimensions'; | ||
| import styles from '../../../../styles/styles'; | ||
| import FixedFooter from '../../../../components/FixedFooter'; | ||
| import Button from '../../../../components/Button'; | ||
|
|
@@ -24,6 +26,7 @@ import * as TwoFactorAuthActions from '../../../../libs/actions/TwoFactorAuthAct | |
|
|
||
| const propTypes = { | ||
| ...withLocalizePropTypes, | ||
| ...windowDimensionsPropTypes, | ||
| account: PropTypes.shape({ | ||
| /** User recovery codes for setting up 2-FA */ | ||
| recoveryCodes: PropTypes.string, | ||
|
|
@@ -64,65 +67,67 @@ function CodesPage(props) { | |
| onCloseButtonPress={() => Navigation.dismissModal(true)} | ||
| /> | ||
| <FullPageOfflineBlockingView> | ||
| <ScrollView contentContainerStyle={styles.flex1}> | ||
| <Section | ||
| title={props.translate('twoFactorAuth.keepCodesSafe')} | ||
| icon={Illustrations.ShieldYellow} | ||
| containerStyles={[styles.twoFactorAuthSection]} | ||
| iconContainerStyles={[styles.ml6]} | ||
| > | ||
| <View style={styles.mv3}> | ||
| <Text>{props.translate('twoFactorAuth.codesLoseAccess')}</Text> | ||
| </View> | ||
| <View style={[styles.twoFactorAuthCodesBox(props)]}> | ||
| {props.account.isLoading ? ( | ||
| <View style={styles.twoFactorLoadingContainer}> | ||
| <ActivityIndicator color={themeColors.spinner} /> | ||
| </View> | ||
| ) : ( | ||
| <> | ||
| <View style={styles.twoFactorAuthCodesContainer}> | ||
| {Boolean(props.account.recoveryCodes) && | ||
| _.map(props.account.recoveryCodes.split(', '), (code) => ( | ||
| <Text | ||
| style={styles.twoFactorAuthCode} | ||
| key={code} | ||
| > | ||
| {code} | ||
| </Text> | ||
| ))} | ||
| </View> | ||
| <View style={styles.twoFactorAuthCodesButtonsContainer}> | ||
| <Button | ||
| text={props.translate('twoFactorAuth.copyCodes')} | ||
| medium | ||
| onPress={() => { | ||
| Clipboard.setString(props.account.recoveryCodes); | ||
| setIsNextButtonDisabled(false); | ||
| }} | ||
| style={styles.twoFactorAuthCodesButton} | ||
| /> | ||
| <Button | ||
| text={props.translate('common.download')} | ||
| medium | ||
| onPress={() => { | ||
| localFileDownload('two-factor-auth-codes', props.account.recoveryCodes); | ||
| setIsNextButtonDisabled(false); | ||
| }} | ||
| style={styles.twoFactorAuthCodesButton} | ||
| /> | ||
| </View> | ||
| </> | ||
| )} | ||
| </View> | ||
| </Section> | ||
| <FixedFooter style={[styles.twoFactorAuthFooter]}> | ||
| <Button | ||
| success | ||
| text={props.translate('common.next')} | ||
| onPress={() => Navigation.navigate(ROUTES.SETTINGS_2FA_VERIFY)} | ||
| isDisabled={isNextButtonDisabled} | ||
| /> | ||
| </FixedFooter> | ||
|
Comment on lines
+122
to
+129
There was a problem hiding this comment. Seems we accidentally included FixedFooter inside scroll containers and that led to this regression #20138 |
||
| </ScrollView> | ||
| </FullPageOfflineBlockingView> | ||
| </ScreenWrapper> | ||
| ); | ||
|
|
@@ -133,6 +138,7 @@ CodesPage.defaultProps = defaultProps; | |
|
|
||
| export default compose( | ||
| withLocalize, | ||
| withWindowDimensions, | ||
| withOnyx({ | ||
| account: {key: ONYXKEYS.ACCOUNT}, | ||
| }), | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2127,14 +2127,26 @@ const styles = { | |
| padding: 0, | ||
| }, | ||
|
|
||
| twoFactorAuthCodesBox: ({isExtraSmallScreenWidth, isSmallScreenWidth}) => { | ||
| let paddingHorizontal = styles.ph15; | ||
|
There was a problem hiding this comment. @shawnborton buddy check please for these new style There was a problem hiding this comment. What exactly am I reviewing here? Just the screenshots from the first comment? There was a problem hiding this comment. sorry, Nothing has changed much in the view. I wanted to get an eye on padding prooerty we added to support view for extra small devices. There was a problem hiding this comment. Ah got it. Can you show me screenshots of what this looks like in practice please? Thanks! There was a problem hiding this comment. @shawnborton Here you go: Desktop, using padding from Figma: `styles.ph15`
Small screen width (<= 800px), decreased padding until it doesn't break: `styles.ph10`
Extra small screen width (<= 320px), decreased padding until it doesn't break: `styles.ph4`
There was a problem hiding this comment. Okay cool, that works for me! |
||
|
|
||
| if (isSmallScreenWidth) { | ||
| paddingHorizontal = styles.ph10; | ||
| } | ||
|
|
||
| if (isExtraSmallScreenWidth) { | ||
| paddingHorizontal = styles.ph4; | ||
| } | ||
|
|
||
| return { | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| backgroundColor: themeColors.highlightBG, | ||
| paddingVertical: 28, | ||
| borderRadius: 16, | ||
| marginTop: 32, | ||
| ...paddingHorizontal, | ||
| }; | ||
| }, | ||
|
|
||
| twoFactorLoadingContainer: { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Thanks! Is this file really needed?
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 just merged
mainand installed dependencies. I guess in some other PR the dependencies were updated but the lockfile wasn't pushed