From 5b66bdc51392d7887fd9e70618db7ea5c4755f9f Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 16 Sep 2021 00:53:49 +0530 Subject: [PATCH 01/23] make Input accessible --- .../ExpensiTextInput/BaseExpensiTextInput.js | 8 +++ .../ExpensiTextInputLabel/index.js | 60 ++++++++++++------- .../ExpensiTextInputLabel/propTypes.js | 14 ++++- src/components/ExpensiTextInput/propTypes.js | 4 ++ src/libs/TextInputUtils/index.js | 23 +++++++ src/libs/TextInputUtils/index.native.js | 10 ++++ 6 files changed, 94 insertions(+), 25 deletions(-) create mode 100644 src/libs/TextInputUtils/index.js create mode 100644 src/libs/TextInputUtils/index.native.js diff --git a/src/components/ExpensiTextInput/BaseExpensiTextInput.js b/src/components/ExpensiTextInput/BaseExpensiTextInput.js index e955e37a012d..a3de5864c077 100644 --- a/src/components/ExpensiTextInput/BaseExpensiTextInput.js +++ b/src/components/ExpensiTextInput/BaseExpensiTextInput.js @@ -8,6 +8,7 @@ import Text from '../Text'; import {propTypes, defaultProps} from './propTypes'; import themeColors from '../../styles/themes/default'; import styles from '../../styles/styles'; +import {setNativePropsWeb} from '../../libs/TextInputUtils'; const ACTIVE_LABEL_TRANSLATE_Y = -12; const ACTIVE_LABEL_TRANSLATE_X = (translateX = -22) => translateX; @@ -44,6 +45,9 @@ class BaseExpensiTextInput extends Component { if (this.props.autoFocus && this.input) { this.input.focus(); } + if (this.input && this.props.name) { + setNativePropsWeb(this.input, 'name', this.props.name); + } } onFocus() { @@ -120,6 +124,9 @@ class BaseExpensiTextInput extends Component { ignoreLabelTranslateX, innerRef, autoFocus, + + // Only present for Web + name, multiline, ...inputProps } = this.props; @@ -152,6 +159,7 @@ class BaseExpensiTextInput extends Component { } labelTranslateY={this.state.labelTranslateY} labelScale={this.state.labelScale} + for={this.props.nativeID} /> ) : null} ( - - {label} - -); +class ExpensiTextInputLabel extends Component { + constructor(props) { + super(props); + this.label = null; + } + + componentDidMount() { + if (this.label && this.props.for) { + setNativePropsWeb(this.label, 'for', this.props.for); + } + } + + render() { + return ( + this.label = el} + style={[ + styles.expensiTextInputLabel, + styles.expensiTextInputLabelDesktop, + styles.expensiTextInputLabelTransformation( + this.props.labelTranslateY, + this.props.labelTranslateX, + this.props.labelScale, + ), + ]} + > + {this.props.label} + + ); + } +} ExpensiTextInputLabel.propTypes = propTypes; +ExpensiTextInputLabel.defaultProps = defaultProps; ExpensiTextInputLabel.displayName = 'ExpensiTextInputLabel'; -export default memo(ExpensiTextInputLabel); +export default ExpensiTextInputLabel; diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/propTypes.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/propTypes.js index cfaf5f14d37c..9896f7a9e3f7 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/propTypes.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/propTypes.js @@ -3,7 +3,7 @@ import {Animated} from 'react-native'; const propTypes = { /** Label */ - label: PropTypes.string, + label: PropTypes.string.isRequired, /** Label vertical translate */ labelTranslateY: PropTypes.instanceOf(Animated.Value).isRequired, @@ -13,6 +13,16 @@ const propTypes = { /** Label scale */ labelScale: PropTypes.instanceOf(Animated.Value).isRequired, + + /** For attribute for label (only Web) */ + for: PropTypes.string, +}; + +const defaultProps = { + for: '', }; -export default propTypes; +export { + propTypes, + defaultProps, +}; diff --git a/src/components/ExpensiTextInput/propTypes.js b/src/components/ExpensiTextInput/propTypes.js index e3a42808a3d5..4257b703e10b 100644 --- a/src/components/ExpensiTextInput/propTypes.js +++ b/src/components/ExpensiTextInput/propTypes.js @@ -4,6 +4,9 @@ const propTypes = { /** Input label */ label: PropTypes.string, + /** Name attribute for the input (only Web) */ + name: PropTypes.string, + /** Input value */ value: PropTypes.string.isRequired, @@ -31,6 +34,7 @@ const propTypes = { const defaultProps = { label: '', + name: '', errorText: '', placeholder: '', error: false, diff --git a/src/libs/TextInputUtils/index.js b/src/libs/TextInputUtils/index.js new file mode 100644 index 000000000000..a681fd11670d --- /dev/null +++ b/src/libs/TextInputUtils/index.js @@ -0,0 +1,23 @@ +function getPasswordAutocompleteType() { + return 'current-password'; +} + +/** + * Set input Native Props on Web Platform + * + * @param {Object} input + * @param {String} prop + * @param {any} value + */ +function setNativePropsWeb(input, prop, value) { + if (input) { + input.setNativeProps({ + [prop]: value, + }); + } +} + +export { + getPasswordAutocompleteType, + setNativePropsWeb, +}; diff --git a/src/libs/TextInputUtils/index.native.js b/src/libs/TextInputUtils/index.native.js new file mode 100644 index 000000000000..76cb5d3f8abb --- /dev/null +++ b/src/libs/TextInputUtils/index.native.js @@ -0,0 +1,10 @@ +function getPasswordAutocompleteType() { + return 'password'; +} + +function setNativePropsWeb() {} + +export { + getPasswordAutocompleteType, + setNativePropsWeb, +}; From 8fcf1fdcffe61962af28cc5b4845a275e173b629 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 16 Sep 2021 00:54:47 +0530 Subject: [PATCH 02/23] fix: input for password Managers --- src/pages/signin/LoginForm.js | 4 +++- src/pages/signin/PasswordForm.js | 19 ++++++++++++++++++- src/styles/styles.js | 7 +++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index e3b4bfd3f09d..f07e24122cd6 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -75,8 +75,10 @@ class LoginForm extends React.Component { this.setState({login: text})} onSubmitEditing={this.validateAndSubmitForm} autoCapitalize="none" diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index ff2b4cd69950..73f3acd8d52c 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -16,6 +16,7 @@ import ChangeExpensifyLoginLink from './ChangeExpensifyLoginLink'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import compose from '../../libs/compose'; import ExpensiTextInput from '../../components/ExpensiTextInput'; +import {getPasswordAutocompleteType} from '../../libs/TextInputUtils'; const propTypes = { /* Onyx Props */ @@ -82,11 +83,26 @@ class PasswordForm extends React.Component { return ( <> + + + this.setState({password: text})} onSubmitEditing={this.validateAndSubmitForm} @@ -152,5 +168,6 @@ export default compose( withLocalize, withOnyx({ account: {key: ONYXKEYS.ACCOUNT}, + credentials: {key: ONYXKEYS.CREDENTIALS}, }), )(PasswordForm); diff --git a/src/styles/styles.js b/src/styles/styles.js index fbb6a8788bc7..c72dc853ecae 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -312,6 +312,13 @@ const styles = { width: variables.componentSizeNormal, }, + visuallyHidden: { + ...visibility('hidden'), + overflow: 'hidden', + width: 0, + height: 0, + }, + loadingVBAAnimation: { width: 160, height: 160, From 228b1b22943b3bee10adb252b397b7cb7eed5ff4 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 16 Sep 2021 01:12:22 +0530 Subject: [PATCH 03/23] adjust the Form for Password Managers --- .../SignInPageLayoutNarrow.js | 97 ++++++---- .../SignInPageLayout/SignInPageLayoutWide.js | 172 ++++++++++-------- 2 files changed, 156 insertions(+), 113 deletions(-) diff --git a/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js b/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js index 74d213eff0ca..25290cbcd58c 100755 --- a/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js +++ b/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js @@ -7,6 +7,7 @@ import ExpensifyCashLogo from '../../../components/ExpensifyCashLogo'; import Text from '../../../components/Text'; import TermsAndLicenses from '../TermsAndLicenses'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; +import {setNativePropsWeb} from '../../../libs/TextInputUtils'; const propTypes = { /** The children to show inside the layout */ @@ -19,46 +20,66 @@ const propTypes = { ...withLocalizePropTypes, }; -const SignInPageLayoutNarrow = props => ( - - - - - - +class SignInPageLayoutNarrow extends React.Component { + constructor(props) { + super(props); + this.form = null; + } + + componentDidMount() { + // These native props are needed for Password Managers like LastPass + if (this.form) { + setNativePropsWeb(this.form, 'method', 'post'); + setNativePropsWeb(this.form, 'action', '/'); + } + } + + render() { + return ( + + + + this.form = el} + > + + + + + {this.props.welcomeText} + + {this.props.children} + - - {props.welcomeText} - - {props.children} + + + - - - - - - -); + + ); + } +} SignInPageLayoutNarrow.propTypes = propTypes; SignInPageLayoutNarrow.displayName = 'SignInPageLayoutNarrow'; diff --git a/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js b/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js index 947dfe41603f..1343bcd265fb 100755 --- a/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js +++ b/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js @@ -10,6 +10,7 @@ import TermsAndLicenses from '../TermsAndLicenses'; import CONST from '../../../CONST'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; import TextLink from '../../../components/TextLink'; +import {setNativePropsWeb} from '../../../libs/TextInputUtils'; const propTypes = { /** The children to show inside the layout */ @@ -25,87 +26,108 @@ const propTypes = { ...withLocalizePropTypes, }; -const SignInPageLayoutWide = props => ( - - - - - - - - - - {props.welcomeText} - - - {props.children} +class SignInPageLayoutWide extends React.Component { + constructor(props) { + super(props); + this.form = null; + } + + componentDidMount() { + // These native props are needed for Password Managers like LastPass + if (this.form) { + setNativePropsWeb(this.form, 'method', 'post'); + setNativePropsWeb(this.form, 'action', '/'); + } + } + + render() { + return ( + + + + + + + + + + {this.props.welcomeText} + + this.form = el} + > + {this.props.children} + + + + + - - - - - - - - - {props.translate('signInPage.heroHeading')} - - {props.translate('signInPage.heroDescription.phrase1')} - {'\n\n'} - {props.translate('signInPage.heroDescription.phrase2')} - {' '} - - - {props.translate('signInPage.heroDescription.phrase3')} - - - {'. '} - {props.translate('signInPage.heroDescription.phrase4')} - {' '} - - - {props.translate('signInPage.heroDescription.phrase5')} - - + + + + {this.props.translate('signInPage.heroHeading')} + + {this.props.translate('signInPage.heroDescription.phrase1')} + {'\n\n'} + {this.props.translate('signInPage.heroDescription.phrase2')} + {' '} + + + {this.props.translate('signInPage.heroDescription.phrase3')} + + + {'. '} + {this.props.translate('signInPage.heroDescription.phrase4')} + {' '} + + + {this.props.translate('signInPage.heroDescription.phrase5')} + + - . - + . + + + + + + - - - - - -); + ); + } +} SignInPageLayoutWide.propTypes = propTypes; SignInPageLayoutWide.displayName = 'SignInPageLayoutWide'; From 0c95acffcbe840cca1cc57b51a11b59e15c2ac4b Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sun, 7 Nov 2021 12:27:22 +0530 Subject: [PATCH 04/23] Refactor code --- .../ExpensiTextInput/BaseExpensiTextInput.js | 12 ++++--- .../ExpensiTextInputLabel/index.js | 7 ++--- .../ExpensiTextInputLabel/index.native.js | 3 +- src/libs/TextInputUtils/index.js | 31 ++++++++++--------- src/libs/TextInputUtils/index.native.js | 13 ++++---- src/pages/signin/PasswordForm.js | 6 ++-- .../SignInPageLayoutNarrow.js | 6 ++-- .../SignInPageLayout/SignInPageLayoutWide.js | 6 ++-- 8 files changed, 45 insertions(+), 39 deletions(-) diff --git a/src/components/ExpensiTextInput/BaseExpensiTextInput.js b/src/components/ExpensiTextInput/BaseExpensiTextInput.js index f6615ad5b86c..ab4276010d54 100644 --- a/src/components/ExpensiTextInput/BaseExpensiTextInput.js +++ b/src/components/ExpensiTextInput/BaseExpensiTextInput.js @@ -8,7 +8,7 @@ import ExpensiTextInputLabel from './ExpensiTextInputLabel'; import {propTypes, defaultProps} from './baseExpensiTextInputPropTypes'; import themeColors from '../../styles/themes/default'; import styles from '../../styles/styles'; -import {setNativePropsWeb} from '../../libs/TextInputUtils'; +import {setBrowserAttributes} from '../../libs/TextInputUtils'; import InlineErrorText from '../InlineErrorText'; const ACTIVE_LABEL_TRANSLATE_Y = -12; @@ -43,12 +43,16 @@ class BaseExpensiTextInput extends Component { } componentDidMount() { + if (!this.input) { + return; + } + // We are manually managing focus to prevent this issue: https://github.com/Expensify/App/issues/4514 - if (this.props.autoFocus && this.input) { + if (this.props.autoFocus) { this.input.focus(); } - if (this.input && this.props.name) { - setNativePropsWeb(this.input, 'name', this.props.name); + if (this.props.name) { + setBrowserAttributes(this.input, 'name', this.props.name); } } diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js index 39d8f97beaac..8dd0543b462f 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js @@ -1,6 +1,6 @@ import React, {Component} from 'react'; import {Animated} from 'react-native'; -import {setNativePropsWeb} from '../../../libs/TextInputUtils'; +import {setBrowserAttributes} from '../../../libs/TextInputUtils'; import styles from '../../../styles/styles'; import {propTypes, defaultProps} from './expensiTextInputLabelPropTypes'; @@ -11,8 +11,8 @@ class ExpensiTextInputLabel extends Component { } componentDidMount() { - if (this.label && this.props.for) { - setNativePropsWeb(this.label, 'for', this.props.for); + if (this.props.for) { + setBrowserAttributes(this.label, 'for', this.props.for); } } @@ -40,6 +40,5 @@ class ExpensiTextInputLabel extends Component { ExpensiTextInputLabel.propTypes = propTypes; ExpensiTextInputLabel.defaultProps = defaultProps; -ExpensiTextInputLabel.displayName = 'ExpensiTextInputLabel'; export default ExpensiTextInputLabel; diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js index 082333f55254..0f122fb234c0 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js @@ -1,7 +1,7 @@ import React, {memo} from 'react'; import {Animated} from 'react-native'; import styles from '../../../styles/styles'; -import propTypes from './expensiTextInputLabelPropTypes'; +import {propTypes, defaultProps} from './expensiTextInputLabelPropTypes'; const ExpensiTextInputLabel = ({ label, @@ -24,6 +24,7 @@ const ExpensiTextInputLabel = ({ ); ExpensiTextInputLabel.propTypes = propTypes; +ExpensiTextInputLabel.propTypes = defaultProps; ExpensiTextInputLabel.displayName = 'ExpensiTextInputLabel'; export default memo(ExpensiTextInputLabel); diff --git a/src/libs/TextInputUtils/index.js b/src/libs/TextInputUtils/index.js index a681fd11670d..550ab3106c87 100644 --- a/src/libs/TextInputUtils/index.js +++ b/src/libs/TextInputUtils/index.js @@ -1,23 +1,24 @@ -function getPasswordAutocompleteType() { - return 'current-password'; -} +/** + * Web password field needs `current-password` as autocomplete type which is not supported on native + */ +const passwordAutocompleteType = 'current-password'; /** - * Set input Native Props on Web Platform - * - * @param {Object} input - * @param {String} prop - * @param {any} value + * Used to set attributes on underlying dom node that are only available on web and throws error on native platform. + * @param {Object} element + * @param {String} attribute + * @param {String} value */ -function setNativePropsWeb(input, prop, value) { - if (input) { - input.setNativeProps({ - [prop]: value, - }); +function setBrowserAttributes(element, attribute, value) { + if (!element) { + return; } + element.setNativeProps({ + [attribute]: value, + }); } export { - getPasswordAutocompleteType, - setNativePropsWeb, + passwordAutocompleteType, + setBrowserAttributes, }; diff --git a/src/libs/TextInputUtils/index.native.js b/src/libs/TextInputUtils/index.native.js index 76cb5d3f8abb..97e6675c68ea 100644 --- a/src/libs/TextInputUtils/index.native.js +++ b/src/libs/TextInputUtils/index.native.js @@ -1,10 +1,11 @@ -function getPasswordAutocompleteType() { - return 'password'; -} +const passwordAutocompleteType = 'password'; -function setNativePropsWeb() {} +/** + * Skip adding browser only attributes to underlying Nodes which will result in an runtime error. + */ +function setBrowserAttributes() {} export { - getPasswordAutocompleteType, - setNativePropsWeb, + passwordAutocompleteType, + setBrowserAttributes, }; diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index a3703f0e5b5f..981b68ad69cd 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -16,7 +16,7 @@ import ChangeExpensifyLoginLink from './ChangeExpensifyLoginLink'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import compose from '../../libs/compose'; import ExpensiTextInput from '../../components/ExpensiTextInput'; -import {getPasswordAutocompleteType} from '../../libs/TextInputUtils'; +import {passwordAutocompleteType} from '../../libs/TextInputUtils'; const propTypes = { /* Onyx Props */ @@ -99,9 +99,9 @@ class PasswordForm extends React.Component { this.setState({password: text})} diff --git a/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js b/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js index 922feda1aa3f..d1f5aadaa6b7 100755 --- a/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js +++ b/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js @@ -7,7 +7,7 @@ import ExpensifyCashLogo from '../../../components/ExpensifyCashLogo'; import Text from '../../../components/Text'; import TermsAndLicenses from '../TermsAndLicenses'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; -import {setNativePropsWeb} from '../../../libs/TextInputUtils'; +import {setBrowserAttributes} from '../../../libs/TextInputUtils'; const propTypes = { /** The children to show inside the layout */ @@ -32,8 +32,8 @@ class SignInPageLayoutNarrow extends React.Component { componentDidMount() { // These native props are needed for Password Managers like LastPass if (this.form) { - setNativePropsWeb(this.form, 'method', 'post'); - setNativePropsWeb(this.form, 'action', '/'); + setBrowserAttributes(this.form, 'method', 'post'); + setBrowserAttributes(this.form, 'action', '/'); } } diff --git a/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js b/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js index be969777496b..d5f8e1b47954 100755 --- a/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js +++ b/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js @@ -8,7 +8,7 @@ import Text from '../../../components/Text'; import variables from '../../../styles/variables'; import TermsAndLicenses from '../TermsAndLicenses'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; -import {setNativePropsWeb} from '../../../libs/TextInputUtils'; +import {setBrowserAttributes} from '../../../libs/TextInputUtils'; const propTypes = { /** The children to show inside the layout */ @@ -38,8 +38,8 @@ class SignInPageLayoutWide extends React.Component { componentDidMount() { // These native props are needed for Password Managers like LastPass if (this.form) { - setNativePropsWeb(this.form, 'method', 'post'); - setNativePropsWeb(this.form, 'action', '/'); + setBrowserAttributes(this.form, 'method', 'post'); + setBrowserAttributes(this.form, 'action', '/'); } } From 08a441ebaf72c43e3ee3e96cd13d511c78ce86ce Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sun, 7 Nov 2021 13:11:42 +0530 Subject: [PATCH 05/23] Refactor Form --- .../ExpensiTextInput/BaseExpensiTextInput.js | 2 +- src/components/Form.js | 25 ++++ .../SignInPageLayoutNarrow.js | 104 +++++++---------- .../SignInPageLayout/SignInPageLayoutWide.js | 109 +++++++----------- 4 files changed, 111 insertions(+), 129 deletions(-) create mode 100644 src/components/Form.js diff --git a/src/components/ExpensiTextInput/BaseExpensiTextInput.js b/src/components/ExpensiTextInput/BaseExpensiTextInput.js index ab4276010d54..6ba04798c8e9 100644 --- a/src/components/ExpensiTextInput/BaseExpensiTextInput.js +++ b/src/components/ExpensiTextInput/BaseExpensiTextInput.js @@ -157,10 +157,10 @@ class BaseExpensiTextInput extends Component { ignoreLabelTranslateX, innerRef, autoFocus, + multiline, // Only present for Web name, - multiline, ...inputProps } = this.props; diff --git a/src/components/Form.js b/src/components/Form.js new file mode 100644 index 000000000000..1f55278c56e5 --- /dev/null +++ b/src/components/Form.js @@ -0,0 +1,25 @@ +import React from 'react'; +import {View} from 'react-native'; +import {setBrowserAttributes} from '../libs/TextInputUtils'; + +class Form extends React.Component { + componentDidMount() { + setBrowserAttributes(this.form, 'method', 'post'); + setBrowserAttributes(this.form, 'action', '/'); + } + + render() { + return ( + this.form = el} + // eslint-disable-next-line react/jsx-props-no-spreading + {...this.props} + /> + ); + } +} + +Form.displayName = 'Form'; +export default Form; diff --git a/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js b/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js index d1f5aadaa6b7..baf5b91cfa10 100755 --- a/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js +++ b/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js @@ -7,7 +7,7 @@ import ExpensifyCashLogo from '../../../components/ExpensifyCashLogo'; import Text from '../../../components/Text'; import TermsAndLicenses from '../TermsAndLicenses'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; -import {setBrowserAttributes} from '../../../libs/TextInputUtils'; +import Form from '../../../components/Form'; const propTypes = { /** The children to show inside the layout */ @@ -23,68 +23,48 @@ const propTypes = { ...withLocalizePropTypes, }; -class SignInPageLayoutNarrow extends React.Component { - constructor(props) { - super(props); - this.form = null; - } - - componentDidMount() { - // These native props are needed for Password Managers like LastPass - if (this.form) { - setBrowserAttributes(this.form, 'method', 'post'); - setBrowserAttributes(this.form, 'action', '/'); - } - } - - render() { - return ( - - - - this.form = el} - > - - - - {this.props.shouldShowWelcomeText && ( - - {this.props.welcomeText} - - )} - {this.props.children} - +const SignInPageLayoutNarrow = props => ( + + + +
+ + - - - - - - ); - } -} + {props.shouldShowWelcomeText && ( + + {props.welcomeText} + + )} + {props.children} +
+ + + + + + +); SignInPageLayoutNarrow.propTypes = propTypes; SignInPageLayoutNarrow.displayName = 'SignInPageLayoutNarrow'; diff --git a/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js b/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js index d5f8e1b47954..de9e8f770923 100755 --- a/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js +++ b/src/pages/signin/SignInPageLayout/SignInPageLayoutWide.js @@ -8,7 +8,7 @@ import Text from '../../../components/Text'; import variables from '../../../styles/variables'; import TermsAndLicenses from '../TermsAndLicenses'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; -import {setBrowserAttributes} from '../../../libs/TextInputUtils'; +import Form from '../../../components/Form'; const propTypes = { /** The children to show inside the layout */ @@ -29,76 +29,53 @@ const propTypes = { const backgroundStyle = getLoginPagePromoStyle(); -class SignInPageLayoutWide extends React.Component { - constructor(props) { - super(props); - this.form = null; - } - - componentDidMount() { - // These native props are needed for Password Managers like LastPass - if (this.form) { - setBrowserAttributes(this.form, 'method', 'post'); - setBrowserAttributes(this.form, 'action', '/'); - } - } - - render() { - return ( - - - - - - - - - {this.props.shouldShowWelcomeText && ( - - {this.props.welcomeText} - - )} - this.form = el} - > - {this.props.children} - - - - - +const SignInPageLayoutWide = props => ( + + + + + + + + {props.shouldShowWelcomeText && ( + + {props.welcomeText} + + )} +
{props.children}
 - - + + - ); - } -} + + + + + +); SignInPageLayoutWide.propTypes = propTypes; SignInPageLayoutWide.displayName = 'SignInPageLayoutWide'; From f5c17093645ac72b90dbf4bab912bf5349a1cc50 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sun, 7 Nov 2021 13:19:52 +0530 Subject: [PATCH 06/23] last touch up --- src/components/ExpensiTextInput/BaseExpensiTextInput.js | 1 - .../ExpensiTextInput/ExpensiTextInputLabel/index.js | 4 ++-- src/pages/signin/PasswordForm.js | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/ExpensiTextInput/BaseExpensiTextInput.js b/src/components/ExpensiTextInput/BaseExpensiTextInput.js index 6ba04798c8e9..3e5de2fef090 100644 --- a/src/components/ExpensiTextInput/BaseExpensiTextInput.js +++ b/src/components/ExpensiTextInput/BaseExpensiTextInput.js @@ -196,7 +196,6 @@ class BaseExpensiTextInput extends Component { labelTranslateY={this.state.labelTranslateY} labelScale={this.state.labelScale} for={this.props.nativeID} - /> ) : null} diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js index 8dd0543b462f..4f6802bd5806 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js @@ -1,10 +1,10 @@ -import React, {Component} from 'react'; +import React, {PureComponent} from 'react'; import {Animated} from 'react-native'; import {setBrowserAttributes} from '../../../libs/TextInputUtils'; import styles from '../../../styles/styles'; import {propTypes, defaultProps} from './expensiTextInputLabelPropTypes'; -class ExpensiTextInputLabel extends Component { +class ExpensiTextInputLabel extends PureComponent { constructor(props) { super(props); this.label = null; diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 981b68ad69cd..60aa101d13ef 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -101,7 +101,7 @@ class PasswordForm extends React.Component { secureTextEntry autoCompleteType={passwordAutocompleteType} textContentType="password" - nativeID={passwordAutocompleteType} + nativeID="password" name="password" value={this.state.password} onChangeText={text => this.setState({password: text})} From 28b22f1c889af411bdb01d228df2aceb562be326 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sun, 7 Nov 2021 16:24:00 +0530 Subject: [PATCH 07/23] Enable multi steo form --- src/pages/signin/LoginForm.js | 9 ++++++--- src/pages/signin/PasswordForm.js | 24 +++++++----------------- src/pages/signin/SignInPage.js | 6 ++---- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index 109189b34459..ec5702fd11aa 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -32,6 +32,9 @@ const propTypes = { loading: PropTypes.bool, }), + /** Whether the page is visible. */ + visible: PropTypes.bool, + ...windowDimensionsPropTypes, ...withLocalizePropTypes, @@ -39,6 +42,7 @@ const propTypes = { const defaultProps = { account: {}, + visible: false, }; class LoginForm extends React.Component { @@ -98,7 +102,7 @@ class LoginForm extends React.Component { render() { return ( - <> + - - + ); } } diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 60aa101d13ef..02e6a504f9c3 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -33,11 +33,15 @@ const propTypes = { loading: PropTypes.bool, }), + /** Whether the page is visible. */ + visible: PropTypes.bool, + ...withLocalizePropTypes, }; const defaultProps = { account: {}, + visible: false, }; class PasswordForm extends React.Component { @@ -81,21 +85,8 @@ class PasswordForm extends React.Component { render() { return ( - <> + - - - - + {this.props.visible && } - + ); } } @@ -172,6 +163,5 @@ export default compose( withLocalize, withOnyx({ account: {key: ONYXKEYS.ACCOUNT}, - credentials: {key: ONYXKEYS.CREDENTIALS}, }), )(PasswordForm); diff --git a/src/pages/signin/SignInPage.js b/src/pages/signin/SignInPage.js index 407f757c7aac..7e1f177964d3 100644 --- a/src/pages/signin/SignInPage.js +++ b/src/pages/signin/SignInPage.js @@ -88,10 +88,8 @@ class SignInPage extends Component { shouldShowWelcomeText={showLoginForm || showPasswordForm || !showResendValidationLinkForm} shouldShowWelcomeScreenshot={showLoginForm} > - {showLoginForm && } - - {showPasswordForm && } - + + {showResendValidationLinkForm && } From bf7859eee8ed070fc920fe4330b4a2c60eca8062 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sun, 7 Nov 2021 16:34:31 +0530 Subject: [PATCH 08/23] Multi step form support --- src/components/Form.js | 4 ++-- src/libs/TextInputUtils/index.js | 2 ++ src/libs/TextInputUtils/index.native.js | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/Form.js b/src/components/Form.js index 1f55278c56e5..7decb19e7c40 100644 --- a/src/components/Form.js +++ b/src/components/Form.js @@ -1,6 +1,6 @@ import React from 'react'; import {View} from 'react-native'; -import {setBrowserAttributes} from '../libs/TextInputUtils'; +import {accessibilityRoleForm, setBrowserAttributes} from '../libs/TextInputUtils'; class Form extends React.Component { componentDidMount() { @@ -11,7 +11,7 @@ class Form extends React.Component { render() { return ( this.form = el} // eslint-disable-next-line react/jsx-props-no-spreading diff --git a/src/libs/TextInputUtils/index.js b/src/libs/TextInputUtils/index.js index 550ab3106c87..763c53f110e0 100644 --- a/src/libs/TextInputUtils/index.js +++ b/src/libs/TextInputUtils/index.js @@ -2,6 +2,7 @@ * Web password field needs `current-password` as autocomplete type which is not supported on native */ const passwordAutocompleteType = 'current-password'; +const accessibilityRoleForm = 'form'; /** * Used to set attributes on underlying dom node that are only available on web and throws error on native platform. @@ -20,5 +21,6 @@ function setBrowserAttributes(element, attribute, value) { export { passwordAutocompleteType, + accessibilityRoleForm, setBrowserAttributes, }; diff --git a/src/libs/TextInputUtils/index.native.js b/src/libs/TextInputUtils/index.native.js index 97e6675c68ea..9fc6b56708af 100644 --- a/src/libs/TextInputUtils/index.native.js +++ b/src/libs/TextInputUtils/index.native.js @@ -1,4 +1,5 @@ const passwordAutocompleteType = 'password'; +const accessibilityRoleForm = 'none'; /** * Skip adding browser only attributes to underlying Nodes which will result in an runtime error. @@ -7,5 +8,6 @@ function setBrowserAttributes() {} export { passwordAutocompleteType, + accessibilityRoleForm, setBrowserAttributes, }; From b180d801a2e4b48c0146e4915a89452715329a3d Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Mon, 8 Nov 2021 22:37:06 +0530 Subject: [PATCH 09/23] refactor page to set the focus correctly --- src/pages/signin/LoginForm.js | 21 +++++++++++++++++++-- src/pages/signin/PasswordForm.js | 16 ++++++++++++++-- src/pages/signin/SignInPage.js | 9 +++++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index ec5702fd11aa..160fabc1d909 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -48,7 +48,7 @@ const defaultProps = { class LoginForm extends React.Component { constructor(props) { super(props); - + this.input = null; this.onTextInput = this.onTextInput.bind(this); this.validateAndSubmitForm = this.validateAndSubmitForm.bind(this); @@ -58,6 +58,23 @@ class LoginForm extends React.Component { }; } + componentDidMount() { + if (canFocusInputOnScreenFocus() && this.input) { + this.input.focus(); + } + } + + componentDidUpdate(prevProps) { + if (!prevProps.visible && this.props.visible) { + this.input.focus(); + + if (this.state.login) { + // eslint-disable-next-line react/no-did-update-set-state + this.setState({login: ''}); + } + } + } + /** * Handle text input and clear formError upon text change * @@ -105,6 +122,7 @@ class LoginForm extends React.Component { this.input = el} label={this.props.translate('loginForm.phoneOrEmail')} value={this.state.login} autoCompleteType="username" @@ -116,7 +134,6 @@ class LoginForm extends React.Component { autoCapitalize="none" autoCorrect={false} keyboardType={getEmailKeyboardType()} - autoFocus={canFocusInputOnScreenFocus()} translateX={-18} /> diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 02e6a504f9c3..12673c110767 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -47,7 +47,7 @@ const defaultProps = { class PasswordForm extends React.Component { constructor(props) { super(props); - + this.input = null; this.validateAndSubmitForm = this.validateAndSubmitForm.bind(this); this.state = { @@ -57,6 +57,18 @@ class PasswordForm extends React.Component { }; } + componentDidMount() { + if (this.input) { + this.input.focus(); + } + } + + componentDidUpdate(prevProps) { + if (!prevProps.visible && this.props.visible) { + this.input.focus(); + } + } + /** * Check that all the form fields are valid, then trigger the submit callback */ @@ -88,6 +100,7 @@ class PasswordForm extends React.Component { this.input = el} label={this.props.translate('common.password')} secureTextEntry autoCompleteType={passwordAutocompleteType} @@ -97,7 +110,6 @@ class PasswordForm extends React.Component { value={this.state.password} onChangeText={text => this.setState({password: text})} onSubmitEditing={this.validateAndSubmitForm} - autoFocus translateX={-18} blurOnSubmit={false} /> diff --git a/src/pages/signin/SignInPage.js b/src/pages/signin/SignInPage.js index 7e1f177964d3..f3f8f9c2778e 100644 --- a/src/pages/signin/SignInPage.js +++ b/src/pages/signin/SignInPage.js @@ -88,8 +88,13 @@ class SignInPage extends Component { shouldShowWelcomeText={showLoginForm || showPasswordForm || !showResendValidationLinkForm} shouldShowWelcomeScreenshot={showLoginForm} > - - + {!showResendValidationLinkForm + && ( + <> + + + + )} {showResendValidationLinkForm && } From e5b81eb0003850e02474a55995d985e56be84198 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Mon, 8 Nov 2021 23:01:33 +0530 Subject: [PATCH 10/23] fix: Dom change --- src/pages/signin/SignInPage.js | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/pages/signin/SignInPage.js b/src/pages/signin/SignInPage.js index f3f8f9c2778e..afcc8d2e1083 100644 --- a/src/pages/signin/SignInPage.js +++ b/src/pages/signin/SignInPage.js @@ -81,24 +81,17 @@ class SignInPage extends Component { const welcomeText = this.props.translate(`welcomeText.${showPasswordForm ? 'phrase4' : 'phrase1'}`); return ( - <> - - - {!showResendValidationLinkForm - && ( - <> - - - - )} - {showResendValidationLinkForm && } - - - + + + + + {showResendValidationLinkForm && } + + ); } } From 9f035f154dcac4dcb4b77199c4ddc80aaaffd591 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Tue, 9 Nov 2021 21:41:09 +0530 Subject: [PATCH 11/23] refactor --- .../ExpensiTextInputLabel/index.js | 5 +++-- src/pages/signin/LoginForm.js | 18 ++++++++++-------- src/pages/signin/PasswordForm.js | 10 ++++++---- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js index 4f6802bd5806..617d551045bc 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js @@ -11,9 +11,10 @@ class ExpensiTextInputLabel extends PureComponent { } componentDidMount() { - if (this.props.for) { - setBrowserAttributes(this.label, 'for', this.props.for); + if (!this.props.for) { + return; } + setBrowserAttributes(this.label, 'for', this.props.for); } render() { diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index 160fabc1d909..bb55a2bccaad 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -59,19 +59,21 @@ class LoginForm extends React.Component { } componentDidMount() { - if (canFocusInputOnScreenFocus() && this.input) { - this.input.focus(); + if (!canFocusInputOnScreenFocus() || !this.input) { + return; } + this.input.focus(); } componentDidUpdate(prevProps) { - if (!prevProps.visible && this.props.visible) { - this.input.focus(); + if (prevProps.visible || !this.props.visible) { + return; + } + this.input.focus(); - if (this.state.login) { - // eslint-disable-next-line react/no-did-update-set-state - this.setState({login: ''}); - } + if (this.state.login) { + // eslint-disable-next-line react/no-did-update-set-state + this.setState({login: ''}); } } diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 12673c110767..678c212a3575 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -58,15 +58,17 @@ class PasswordForm extends React.Component { } componentDidMount() { - if (this.input) { - this.input.focus(); + if (!this.input) { + return; } + this.input.focus(); } componentDidUpdate(prevProps) { - if (!prevProps.visible && this.props.visible) { - this.input.focus(); + if (prevProps.visible || !this.props.visible) { + return; } + this.input.focus(); } /** From 5938bd89d38208053b4a43b5d80bf2e3d34f0f29 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Wed, 1 Dec 2021 18:45:10 +0530 Subject: [PATCH 12/23] Review- 1 --- src/components/Form.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/Form.js b/src/components/Form.js index 7decb19e7c40..14df68d3c155 100644 --- a/src/components/Form.js +++ b/src/components/Form.js @@ -1,17 +1,18 @@ import React from 'react'; import {View} from 'react-native'; -import {accessibilityRoleForm, setBrowserAttributes} from '../libs/TextInputUtils'; +import * as TextInputUtils from '../libs/TextInputUtils'; class Form extends React.Component { componentDidMount() { - setBrowserAttributes(this.form, 'method', 'post'); - setBrowserAttributes(this.form, 'action', '/'); + // Password Managers need these atributes to be able to identify the form elments properly. + TextInputUtils.setBrowserAttributes(this.form, 'method', 'post'); + TextInputUtils.setBrowserAttributes(this.form, 'action', '/'); } render() { return ( this.form = el} // eslint-disable-next-line react/jsx-props-no-spreading @@ -21,5 +22,4 @@ class Form extends React.Component { } } -Form.displayName = 'Form'; export default Form; From 46430c7531a78ce9cd19639fd9a7bcf0483f2cbf Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 2 Dec 2021 21:37:30 +0530 Subject: [PATCH 13/23] Review comments --- src/components/CollapsibleView.js | 28 +++++++++++++++ .../ExpensiTextInput/BaseExpensiTextInput.js | 9 ++--- .../ExpensiTextInputLabel/index.js | 8 +---- src/components/Form/BaseForm.js | 16 +++++++++ src/components/{Form.js => Form/index.js} | 17 +++++---- src/components/Form/index.native.js | 8 +++++ src/components/TextInputWithName/index.js | 36 +++++++++++++++++++ .../TextInputWithName/index.native.js | 20 +++++++++++ .../textInputWithNamepropTypes.js | 19 ++++++++++ src/libs/ComponentUtils/index.js | 10 ++++++ .../index.native.js | 8 ++--- src/libs/TextInputUtils/index.js | 26 -------------- src/pages/signin/LoginForm.js | 23 +++++++----- src/pages/signin/PasswordForm.js | 15 ++++---- src/pages/signin/SignInPage.js | 10 ++++-- 15 files changed, 184 insertions(+), 69 deletions(-) create mode 100644 src/components/CollapsibleView.js create mode 100644 src/components/Form/BaseForm.js rename src/components/{Form.js => Form/index.js} (53%) create mode 100644 src/components/Form/index.native.js create mode 100755 src/components/TextInputWithName/index.js create mode 100644 src/components/TextInputWithName/index.native.js create mode 100644 src/components/TextInputWithName/textInputWithNamepropTypes.js create mode 100644 src/libs/ComponentUtils/index.js rename src/libs/{TextInputUtils => ComponentUtils}/index.native.js (55%) delete mode 100644 src/libs/TextInputUtils/index.js diff --git a/src/components/CollapsibleView.js b/src/components/CollapsibleView.js new file mode 100644 index 000000000000..f46aa3bc2b5c --- /dev/null +++ b/src/components/CollapsibleView.js @@ -0,0 +1,28 @@ +import React from 'react'; +import {View} from 'react-native'; +import PropTypes from 'prop-types'; +import styles from '../styles/styles'; + +const propTypes = { + /** Main Content */ + children: PropTypes.func.isRequired, + + /** Whether the content is visible. */ + isVisible: PropTypes.bool, +}; + +const defaultProps = { + isVisible: false, +}; + +const CollapsibleView = props => ( + + {props.children(props.isVisible)} + +); + +CollapsibleView.propTypes = propTypes; +CollapsibleView.defaultProps = defaultProps; +CollapsibleView.displayName = 'CollapsibleView'; + +export default CollapsibleView; diff --git a/src/components/ExpensiTextInput/BaseExpensiTextInput.js b/src/components/ExpensiTextInput/BaseExpensiTextInput.js index a1932f77f257..472b3d6c521f 100644 --- a/src/components/ExpensiTextInput/BaseExpensiTextInput.js +++ b/src/components/ExpensiTextInput/BaseExpensiTextInput.js @@ -1,18 +1,18 @@ import _ from 'underscore'; import React, {Component} from 'react'; import { - Animated, TextInput, View, TouchableWithoutFeedback, Pressable, + Animated, View, TouchableWithoutFeedback, Pressable, } from 'react-native'; import Str from 'expensify-common/lib/str'; import ExpensiTextInputLabel from './ExpensiTextInputLabel'; import * as baseExpensiTextInputPropTypes from './baseExpensiTextInputPropTypes'; import themeColors from '../../styles/themes/default'; import styles from '../../styles/styles'; -import * as TextInputUtils from '../../libs/TextInputUtils'; import Icon from '../Icon'; import * as Expensicons from '../Icon/Expensicons'; import InlineErrorText from '../InlineErrorText'; import * as styleConst from './styleConst'; +import TextInputWithName from '../TextInputWithName'; class BaseExpensiTextInput extends Component { constructor(props) { @@ -46,9 +46,6 @@ class BaseExpensiTextInput extends Component { if (this.props.autoFocus) { this.input.focus(); } - if (this.props.name) { - TextInputUtils.setBrowserAttributes(this.input, 'name', this.props.name); - } } componentDidUpdate(prevProps) { @@ -178,7 +175,7 @@ class BaseExpensiTextInput extends Component { ) : null} - { if (typeof this.props.innerRef === 'function') { this.props.innerRef(ref); } this.input = ref; diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js index b7c21e7c4358..208f6271be9e 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js @@ -1,20 +1,14 @@ import React, {PureComponent} from 'react'; import {Animated} from 'react-native'; -import * as TextInputUtils from '../../../libs/TextInputUtils'; import styles from '../../../styles/styles'; import {propTypes, defaultProps} from './expensiTextInputLabelPropTypes'; class ExpensiTextInputLabel extends PureComponent { - constructor(props) { - super(props); - this.label = null; - } - componentDidMount() { if (!this.props.for) { return; } - TextInputUtils.setBrowserAttributes(this.label, 'for', this.props.for); + this.label.setNativeProps({for: this.props.for}); } render() { diff --git a/src/components/Form/BaseForm.js b/src/components/Form/BaseForm.js new file mode 100644 index 000000000000..21a4e19b1f9b --- /dev/null +++ b/src/components/Form/BaseForm.js @@ -0,0 +1,16 @@ +import React, {forwardRef} from 'react'; +import {View} from 'react-native'; +import * as ComponentUtils from '../../libs/ComponentUtils'; + +const BaseForm = forwardRef((props, ref) => ( + +)); + +BaseForm.displayName = 'BaseForm'; +export default BaseForm; diff --git a/src/components/Form.js b/src/components/Form/index.js similarity index 53% rename from src/components/Form.js rename to src/components/Form/index.js index 14df68d3c155..de070182dd6d 100644 --- a/src/components/Form.js +++ b/src/components/Form/index.js @@ -1,19 +1,22 @@ import React from 'react'; -import {View} from 'react-native'; -import * as TextInputUtils from '../libs/TextInputUtils'; +import BaseForm from './BaseForm'; class Form extends React.Component { componentDidMount() { + if (!this.form) { + return; + } + // Password Managers need these atributes to be able to identify the form elments properly. - TextInputUtils.setBrowserAttributes(this.form, 'method', 'post'); - TextInputUtils.setBrowserAttributes(this.form, 'action', '/'); + this.form.setNativeProps({ + method: 'post', + action: '/', + }); } render() { return ( - this.form = el} // eslint-disable-next-line react/jsx-props-no-spreading {...this.props} diff --git a/src/components/Form/index.native.js b/src/components/Form/index.native.js new file mode 100644 index 000000000000..21f10e7a428d --- /dev/null +++ b/src/components/Form/index.native.js @@ -0,0 +1,8 @@ +import React from 'react'; +import BaseForm from './BaseForm'; + +// eslint-disable-next-line react/jsx-props-no-spreading +const Form = props => ; + +Form.displayName = 'Form'; +export default Form; diff --git a/src/components/TextInputWithName/index.js b/src/components/TextInputWithName/index.js new file mode 100755 index 000000000000..e7dad050d213 --- /dev/null +++ b/src/components/TextInputWithName/index.js @@ -0,0 +1,36 @@ +import React from 'react'; +import {TextInput} from 'react-native'; +import textInputWithNamepropTypes from './textInputWithNamepropTypes'; + +/** + * On web we need to set the native attribure name for accessiblity. + */ +class TextInputWithName extends React.Component { + componentDidMount() { + if (!this.textInput) { + return; + } + + if (this.props.name) { + this.textInput.setNativeProps({name: this.props.name}); + } + } + + render() { + return ( + + ); + } +} + +TextInputWithName.propTypes = textInputWithNamepropTypes.propTypes; +TextInputWithName.defaultProps = textInputWithNamepropTypes.defaultProps; + +export default React.forwardRef((props, ref) => ( + /* eslint-disable-next-line react/jsx-props-no-spreading */ + +)); diff --git a/src/components/TextInputWithName/index.native.js b/src/components/TextInputWithName/index.native.js new file mode 100644 index 000000000000..198c9540dc2f --- /dev/null +++ b/src/components/TextInputWithName/index.native.js @@ -0,0 +1,20 @@ +import React from 'react'; +import {TextInput} from 'react-native'; +import textInputWithNamepropTypes from './textInputWithNamepropTypes'; + +const TextInputWithName = props => ( + +); + +TextInputWithName.propTypes = textInputWithNamepropTypes.propTypes; +TextInputWithName.defaultProps = textInputWithNamepropTypes.defaultProps; +TextInputWithName.displayName = 'TextInputWithName'; + +export default React.forwardRef((props, ref) => ( + /* eslint-disable-next-line react/jsx-props-no-spreading */ + +)); diff --git a/src/components/TextInputWithName/textInputWithNamepropTypes.js b/src/components/TextInputWithName/textInputWithNamepropTypes.js new file mode 100644 index 000000000000..902ff2289d68 --- /dev/null +++ b/src/components/TextInputWithName/textInputWithNamepropTypes.js @@ -0,0 +1,19 @@ +import PropTypes from 'prop-types'; + +const propTypes = { + /** Name attribute for the input */ + name: PropTypes.string, + + /** A ref to forward to the text input */ + forwardedRef: PropTypes.func, +}; + +const defaultProps = { + name: '', + forwardedRef: () => {}, +}; + +export default { + propTypes, + defaultProps, +}; diff --git a/src/libs/ComponentUtils/index.js b/src/libs/ComponentUtils/index.js new file mode 100644 index 000000000000..f3b1b07f07d8 --- /dev/null +++ b/src/libs/ComponentUtils/index.js @@ -0,0 +1,10 @@ +/** + * Web password field needs `current-password` as autocomplete type which is not supported on native + */ +const PASSWORD_AUTOCOMPLETE_TYPE = 'current-password'; +const ACCESSIBILITY_ROLE_FORM = 'form'; + +export { + PASSWORD_AUTOCOMPLETE_TYPE, + ACCESSIBILITY_ROLE_FORM, +}; diff --git a/src/libs/TextInputUtils/index.native.js b/src/libs/ComponentUtils/index.native.js similarity index 55% rename from src/libs/TextInputUtils/index.native.js rename to src/libs/ComponentUtils/index.native.js index 9fc6b56708af..c48d46df3328 100644 --- a/src/libs/TextInputUtils/index.native.js +++ b/src/libs/ComponentUtils/index.native.js @@ -1,5 +1,5 @@ -const passwordAutocompleteType = 'password'; -const accessibilityRoleForm = 'none'; +const PASSWORD_AUTOCOMPLETE_TYPE = 'password'; +const ACCESSIBILITY_ROLE_FORM = 'none'; /** * Skip adding browser only attributes to underlying Nodes which will result in an runtime error. @@ -7,7 +7,7 @@ const accessibilityRoleForm = 'none'; function setBrowserAttributes() {} export { - passwordAutocompleteType, - accessibilityRoleForm, + PASSWORD_AUTOCOMPLETE_TYPE, + ACCESSIBILITY_ROLE_FORM, setBrowserAttributes, }; diff --git a/src/libs/TextInputUtils/index.js b/src/libs/TextInputUtils/index.js deleted file mode 100644 index 763c53f110e0..000000000000 --- a/src/libs/TextInputUtils/index.js +++ /dev/null @@ -1,26 +0,0 @@ -/** - * Web password field needs `current-password` as autocomplete type which is not supported on native - */ -const passwordAutocompleteType = 'current-password'; -const accessibilityRoleForm = 'form'; - -/** - * Used to set attributes on underlying dom node that are only available on web and throws error on native platform. - * @param {Object} element - * @param {String} attribute - * @param {String} value - */ -function setBrowserAttributes(element, attribute, value) { - if (!element) { - return; - } - element.setNativeProps({ - [attribute]: value, - }); -} - -export { - passwordAutocompleteType, - accessibilityRoleForm, - setBrowserAttributes, -}; diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index a5ba4f7725b2..9f955976d3ef 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -34,7 +34,7 @@ const propTypes = { }), /** Whether the page is visible. */ - visible: PropTypes.bool, + isVisible: PropTypes.bool, ...windowDimensionsPropTypes, @@ -43,16 +43,15 @@ const propTypes = { const defaultProps = { account: {}, - visible: false, + isVisible: false, }; class LoginForm extends React.Component { constructor(props) { super(props); - this.input = null; this.onTextInput = this.onTextInput.bind(this); this.validateAndSubmitForm = this.validateAndSubmitForm.bind(this); - + this.clearLogin = this.clearLogin.bind(this); this.state = { formError: false, login: '', @@ -67,14 +66,13 @@ class LoginForm extends React.Component { } componentDidUpdate(prevProps) { - if (prevProps.visible || !this.props.visible) { + if (prevProps.isVisible || !this.props.isVisible) { return; } this.input.focus(); if (this.state.login) { - // eslint-disable-next-line react/no-did-update-set-state - this.setState({login: ''}); + this.clearLogin(); } } @@ -94,6 +92,13 @@ class LoginForm extends React.Component { } } + /** + * Clear Login from the state + */ + clearLogin() { + this.setState({login: ''}); + } + /** * Check that all the form fields are valid, then trigger the submit callback */ @@ -125,7 +130,7 @@ class LoginForm extends React.Component { render() { return ( - + <> this.input = el} @@ -166,7 +171,7 @@ class LoginForm extends React.Component { onPress={this.validateAndSubmitForm} /> - + ); } } diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 62540054eab1..19601fe81aad 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -16,7 +16,7 @@ import ChangeExpensifyLoginLink from './ChangeExpensifyLoginLink'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import compose from '../../libs/compose'; import ExpensiTextInput from '../../components/ExpensiTextInput'; -import * as TextInputUtils from '../../libs/TextInputUtils'; +import * as ComponentUtils from '../../libs/ComponentUtils'; const propTypes = { /* Onyx Props */ @@ -34,20 +34,19 @@ const propTypes = { }), /** Whether the page is visible. */ - visible: PropTypes.bool, + isVisible: PropTypes.bool, ...withLocalizePropTypes, }; const defaultProps = { account: {}, - visible: false, + isVisible: false, }; class PasswordForm extends React.Component { constructor(props) { super(props); - this.input = null; this.validateAndSubmitForm = this.validateAndSubmitForm.bind(this); this.state = { @@ -65,7 +64,7 @@ class PasswordForm extends React.Component { } componentDidUpdate(prevProps) { - if (prevProps.visible || !this.props.visible) { + if (prevProps.isVisible || !this.props.isVisible) { return; } this.input.focus(); @@ -99,13 +98,13 @@ class PasswordForm extends React.Component { render() { return ( - + <> this.input = el} label={this.props.translate('common.password')} secureTextEntry - autoCompleteType={TextInputUtils.passwordAutocompleteType} + autoCompleteType={ComponentUtils.PASSWORD_AUTOCOMPLETE_TYPE} textContentType="password" nativeID="password" name="password" @@ -163,7 +162,7 @@ class PasswordForm extends React.Component { /> {this.props.visible && } - + ); } } diff --git a/src/pages/signin/SignInPage.js b/src/pages/signin/SignInPage.js index afcc8d2e1083..56599838c920 100644 --- a/src/pages/signin/SignInPage.js +++ b/src/pages/signin/SignInPage.js @@ -13,6 +13,7 @@ import LoginForm from './LoginForm'; import PasswordForm from './PasswordForm'; import ResendValidationForm from './ResendValidationForm'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; +import CollapsibleView from '../../components/CollapsibleView'; const propTypes = { /* Onyx Props */ @@ -87,8 +88,13 @@ class SignInPage extends Component { shouldShowWelcomeText={showLoginForm || showPasswordForm || !showResendValidationLinkForm} shouldShowWelcomeScreenshot={showLoginForm} > - - + {/* Both LoginForm and PasswordForm should be present in the UI for Password Managers to work. */} + + {isVisible => } + + + {isVisible => } + {showResendValidationLinkForm && } From 8b3127a88ff263999e8d7a919ea4b0a0bc1c0f0d Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 2 Dec 2021 21:40:09 +0530 Subject: [PATCH 14/23] fix: Collapsible view --- src/components/CollapsibleView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/CollapsibleView.js b/src/components/CollapsibleView.js index f46aa3bc2b5c..aff3afc14b2e 100644 --- a/src/components/CollapsibleView.js +++ b/src/components/CollapsibleView.js @@ -16,7 +16,7 @@ const defaultProps = { }; const CollapsibleView = props => ( - + {props.children(props.isVisible)} ); From d7365f0530abd4121728e98cab12902fdb841cde Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 2 Dec 2021 21:46:46 +0530 Subject: [PATCH 15/23] fix: form --- .../ExpensiTextInput/BaseExpensiTextInput.js | 28 +++++++++---------- src/pages/signin/PasswordForm.js | 2 +- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/components/ExpensiTextInput/BaseExpensiTextInput.js b/src/components/ExpensiTextInput/BaseExpensiTextInput.js index 472b3d6c521f..84004e0d26d0 100644 --- a/src/components/ExpensiTextInput/BaseExpensiTextInput.js +++ b/src/components/ExpensiTextInput/BaseExpensiTextInput.js @@ -38,14 +38,12 @@ class BaseExpensiTextInput extends Component { } componentDidMount() { - if (!this.input) { + // We are manually managing focus to prevent this issue: https://github.com/Expensify/App/issues/4514 + if (!this.props.autoFocus || !this.input) { return; } - // We are manually managing focus to prevent this issue: https://github.com/Expensify/App/issues/4514 - if (this.props.autoFocus) { - this.input.focus(); - } + this.input.focus(); } componentDidUpdate(prevProps) { @@ -195,16 +193,16 @@ class BaseExpensiTextInput extends Component { onPressOut={this.props.onPress} /> {this.props.secureTextEntry && ( - - - + + + )} diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 19601fe81aad..ee490a6b1c7d 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -160,7 +160,7 @@ class PasswordForm extends React.Component { isLoading={this.props.account.loading} onPress={this.validateAndSubmitForm} /> - {this.props.visible && } + {this.props.isVisible && } ); From 9f5aa7bb1ae361fe57860778eb4f128bbd4d9752 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 2 Dec 2021 21:51:46 +0530 Subject: [PATCH 16/23] pass down the name prop --- src/components/ExpensiTextInput/BaseExpensiTextInput.js | 1 + src/components/TextInputWithName/index.js | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/ExpensiTextInput/BaseExpensiTextInput.js b/src/components/ExpensiTextInput/BaseExpensiTextInput.js index 84004e0d26d0..9394a8d9f88d 100644 --- a/src/components/ExpensiTextInput/BaseExpensiTextInput.js +++ b/src/components/ExpensiTextInput/BaseExpensiTextInput.js @@ -191,6 +191,7 @@ class BaseExpensiTextInput extends Component { onChangeText={this.setValue} secureTextEntry={this.state.passwordHidden} onPressOut={this.props.onPress} + name={this.props.name} /> {this.props.secureTextEntry && ( this.textInput = el} // eslint-disable-next-line react/jsx-props-no-spreading {...this.props} /> From 9703fe6c38c80bb3d9a968abea92d89f49604733 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 2 Dec 2021 22:00:11 +0530 Subject: [PATCH 17/23] typos --- .../ExpensiTextInputLabel/expensiTextInputLabelPropTypes.js | 2 +- .../ExpensiTextInput/baseExpensiTextInputPropTypes.js | 2 +- src/components/Form/index.js | 2 +- src/libs/ComponentUtils/index.native.js | 6 ------ 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/expensiTextInputLabelPropTypes.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/expensiTextInputLabelPropTypes.js index fe773eb3f455..775605b93fc5 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/expensiTextInputLabelPropTypes.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/expensiTextInputLabelPropTypes.js @@ -11,7 +11,7 @@ const propTypes = { /** Label scale */ labelScale: PropTypes.instanceOf(Animated.Value).isRequired, - /** For attribute for label (only Web) */ + /** For attribute for label */ for: PropTypes.string, }; diff --git a/src/components/ExpensiTextInput/baseExpensiTextInputPropTypes.js b/src/components/ExpensiTextInput/baseExpensiTextInputPropTypes.js index 17a5d7249114..6023b6d31d9c 100644 --- a/src/components/ExpensiTextInput/baseExpensiTextInputPropTypes.js +++ b/src/components/ExpensiTextInput/baseExpensiTextInputPropTypes.js @@ -4,7 +4,7 @@ const propTypes = { /** Input label */ label: PropTypes.string, - /** Name attribute for the input (only Web) */ + /** Name attribute for the input */ name: PropTypes.string, /** Input value */ diff --git a/src/components/Form/index.js b/src/components/Form/index.js index de070182dd6d..163f1ee223b1 100644 --- a/src/components/Form/index.js +++ b/src/components/Form/index.js @@ -7,7 +7,7 @@ class Form extends React.Component { return; } - // Password Managers need these atributes to be able to identify the form elments properly. + // Password Managers need these attributes to be able to identify the form elements properly. this.form.setNativeProps({ method: 'post', action: '/', diff --git a/src/libs/ComponentUtils/index.native.js b/src/libs/ComponentUtils/index.native.js index c48d46df3328..fcde8612a2f9 100644 --- a/src/libs/ComponentUtils/index.native.js +++ b/src/libs/ComponentUtils/index.native.js @@ -1,13 +1,7 @@ const PASSWORD_AUTOCOMPLETE_TYPE = 'password'; const ACCESSIBILITY_ROLE_FORM = 'none'; -/** - * Skip adding browser only attributes to underlying Nodes which will result in an runtime error. - */ -function setBrowserAttributes() {} - export { PASSWORD_AUTOCOMPLETE_TYPE, ACCESSIBILITY_ROLE_FORM, - setBrowserAttributes, }; From a1c5ac04aba3c8f7509745511a620df820072889 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 2 Dec 2021 22:16:04 +0530 Subject: [PATCH 18/23] Clear form --- src/pages/signin/LoginForm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index 9f955976d3ef..018090c316f7 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -96,7 +96,7 @@ class LoginForm extends React.Component { * Clear Login from the state */ clearLogin() { - this.setState({login: ''}); + this.setState({login: ''}, this.input.clear); } /** From 530627f5c09a9b17b8c14427d051c9f699b4c47d Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 3 Dec 2021 01:09:08 +0530 Subject: [PATCH 19/23] Review -2 --- src/components/CollapsibleView.js | 28 ----------- src/components/withToggleVisibilityView.js | 58 ++++++++++++++++++++++ src/pages/signin/LoginForm.js | 5 ++ src/pages/signin/PasswordForm.js | 3 ++ src/pages/signin/SignInPage.js | 9 +--- 5 files changed, 68 insertions(+), 35 deletions(-) delete mode 100644 src/components/CollapsibleView.js create mode 100644 src/components/withToggleVisibilityView.js diff --git a/src/components/CollapsibleView.js b/src/components/CollapsibleView.js deleted file mode 100644 index aff3afc14b2e..000000000000 --- a/src/components/CollapsibleView.js +++ /dev/null @@ -1,28 +0,0 @@ -import React from 'react'; -import {View} from 'react-native'; -import PropTypes from 'prop-types'; -import styles from '../styles/styles'; - -const propTypes = { - /** Main Content */ - children: PropTypes.func.isRequired, - - /** Whether the content is visible. */ - isVisible: PropTypes.bool, -}; - -const defaultProps = { - isVisible: false, -}; - -const CollapsibleView = props => ( - - {props.children(props.isVisible)} - -); - -CollapsibleView.propTypes = propTypes; -CollapsibleView.defaultProps = defaultProps; -CollapsibleView.displayName = 'CollapsibleView'; - -export default CollapsibleView; diff --git a/src/components/withToggleVisibilityView.js b/src/components/withToggleVisibilityView.js new file mode 100644 index 000000000000..ac1d15aa8c2d --- /dev/null +++ b/src/components/withToggleVisibilityView.js @@ -0,0 +1,58 @@ +import React, {Component} from 'react'; +import {View} from 'react-native'; +import PropTypes from 'prop-types'; +import styles from '../styles/styles'; +import getComponentDisplayName from '../libs/getComponentDisplayName'; + +const toggleVisibilityViewPropTypes = { + /** Whether the content is viToggleVisibilitysible. */ + isVisible: PropTypes.bool, +}; + +export default function (WrappedComponent) { + class WithToggleVisibilityView extends Component { + componentDidUpdate(prevProps) { + if (prevProps.isVisible || !this.props.isVisible || !this.focusableElement) { + return; + } + this.focusableElement.focus(); + } + + render() { + return ( + + + + + ); + } + } + + WithToggleVisibilityView.displayName = `WithToggleVisibilityView(${getComponentDisplayName(WrappedComponent)})`; + WithToggleVisibilityView.propTypes = { + forwardedRef: PropTypes.oneOfType([ + PropTypes.func, + PropTypes.shape({current: PropTypes.instanceOf(React.Component)}), + ]), + + /** Whether the content is visible. */ + isVisible: PropTypes.bool, + }; + WithToggleVisibilityView.defaultProps = { + forwardedRef: undefined, + isVisible: false, + }; + return React.forwardRef((props, ref) => ( + // eslint-disable-next-line react/jsx-props-no-spreading + + )); +} + +export { + toggleVisibilityViewPropTypes, +}; diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index 018090c316f7..250ec60818af 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -17,6 +17,7 @@ import getEmailKeyboardType from '../../libs/getEmailKeyboardType'; import ExpensiTextInput from '../../components/ExpensiTextInput'; import * as ValidationUtils from '../../libs/ValidationUtils'; import LoginUtil from '../../libs/LoginUtil'; +import withToggleVisibilityView, {toggleVisibilityViewPropTypes} from '../../components/withToggleVisibilityView'; const propTypes = { /* Onyx Props */ @@ -39,6 +40,8 @@ const propTypes = { ...windowDimensionsPropTypes, ...withLocalizePropTypes, + + ...toggleVisibilityViewPropTypes, }; const defaultProps = { @@ -66,6 +69,7 @@ class LoginForm extends React.Component { } componentDidUpdate(prevProps) { + // Whenever page becomes visible focus the input. We are using withToggleVisibilityView to hide the page instread of unmounting. if (prevProps.isVisible || !this.props.isVisible) { return; } @@ -185,4 +189,5 @@ export default compose( }), withWindowDimensions, withLocalize, + withToggleVisibilityView, )(LoginForm); diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index ee490a6b1c7d..ada392e51082 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -17,6 +17,7 @@ import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize import compose from '../../libs/compose'; import ExpensiTextInput from '../../components/ExpensiTextInput'; import * as ComponentUtils from '../../libs/ComponentUtils'; +import withToggleVisibilityView from '../../components/withToggleVisibilityView'; const propTypes = { /* Onyx Props */ @@ -64,6 +65,7 @@ class PasswordForm extends React.Component { } componentDidUpdate(prevProps) { + // Whenever page becomes visible focus the input. We are using withToggleVisibilityView to hide the page instread of unmounting. if (prevProps.isVisible || !this.props.isVisible) { return; } @@ -175,4 +177,5 @@ export default compose( withOnyx({ account: {key: ONYXKEYS.ACCOUNT}, }), + withToggleVisibilityView, )(PasswordForm); diff --git a/src/pages/signin/SignInPage.js b/src/pages/signin/SignInPage.js index 56599838c920..1d50b31c5c35 100644 --- a/src/pages/signin/SignInPage.js +++ b/src/pages/signin/SignInPage.js @@ -13,7 +13,6 @@ import LoginForm from './LoginForm'; import PasswordForm from './PasswordForm'; import ResendValidationForm from './ResendValidationForm'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; -import CollapsibleView from '../../components/CollapsibleView'; const propTypes = { /* Onyx Props */ @@ -89,12 +88,8 @@ class SignInPage extends Component { shouldShowWelcomeScreenshot={showLoginForm} > {/* Both LoginForm and PasswordForm should be present in the UI for Password Managers to work. */} - - {isVisible => } - - - {isVisible => } - + + {showResendValidationLinkForm && } From b59405913345b5d1d564a288508a59cbd76ee57e Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 3 Dec 2021 01:14:12 +0530 Subject: [PATCH 20/23] review-3 --- src/components/TextInputWithName/index.js | 2 +- src/components/withToggleVisibilityView.js | 34 +++++++--------------- src/pages/signin/PasswordForm.js | 3 +- 3 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/components/TextInputWithName/index.js b/src/components/TextInputWithName/index.js index 95ee6b8dd102..5a78089af609 100755 --- a/src/components/TextInputWithName/index.js +++ b/src/components/TextInputWithName/index.js @@ -4,7 +4,7 @@ import {TextInput} from 'react-native'; import textInputWithNamepropTypes from './textInputWithNamepropTypes'; /** - * On web we need to set the native attribure name for accessiblity. + * On web we need to set the native attribute name for accessiblity. */ class TextInputWithName extends React.Component { componentDidMount() { diff --git a/src/components/withToggleVisibilityView.js b/src/components/withToggleVisibilityView.js index ac1d15aa8c2d..42f9fc891293 100644 --- a/src/components/withToggleVisibilityView.js +++ b/src/components/withToggleVisibilityView.js @@ -1,4 +1,4 @@ -import React, {Component} from 'react'; +import React from 'react'; import {View} from 'react-native'; import PropTypes from 'prop-types'; import styles from '../styles/styles'; @@ -10,28 +10,16 @@ const toggleVisibilityViewPropTypes = { }; export default function (WrappedComponent) { - class WithToggleVisibilityView extends Component { - componentDidUpdate(prevProps) { - if (prevProps.isVisible || !this.props.isVisible || !this.focusableElement) { - return; - } - this.focusableElement.focus(); - } - - render() { - return ( - - - - - ); - } - } + const WithToggleVisibilityView = props => ( + + + + ); WithToggleVisibilityView.displayName = `WithToggleVisibilityView(${getComponentDisplayName(WrappedComponent)})`; WithToggleVisibilityView.propTypes = { diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index ada392e51082..a1412fe00316 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -17,7 +17,7 @@ import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize import compose from '../../libs/compose'; import ExpensiTextInput from '../../components/ExpensiTextInput'; import * as ComponentUtils from '../../libs/ComponentUtils'; -import withToggleVisibilityView from '../../components/withToggleVisibilityView'; +import withToggleVisibilityView, {toggleVisibilityViewPropTypes} from '../../components/withToggleVisibilityView'; const propTypes = { /* Onyx Props */ @@ -38,6 +38,7 @@ const propTypes = { isVisible: PropTypes.bool, ...withLocalizePropTypes, + ...toggleVisibilityViewPropTypes, }; const defaultProps = { From db6b4b684b620e553b4c3aa89267a88df33dc985 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 3 Dec 2021 01:18:04 +0530 Subject: [PATCH 21/23] Extra prop definition removed --- src/pages/signin/LoginForm.js | 4 ---- src/pages/signin/PasswordForm.js | 4 ---- 2 files changed, 8 deletions(-) diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index 250ec60818af..494215e63278 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -34,9 +34,6 @@ const propTypes = { loading: PropTypes.bool, }), - /** Whether the page is visible. */ - isVisible: PropTypes.bool, - ...windowDimensionsPropTypes, ...withLocalizePropTypes, @@ -46,7 +43,6 @@ const propTypes = { const defaultProps = { account: {}, - isVisible: false, }; class LoginForm extends React.Component { diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index a1412fe00316..5bb3374f8d36 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -34,16 +34,12 @@ const propTypes = { loading: PropTypes.bool, }), - /** Whether the page is visible. */ - isVisible: PropTypes.bool, - ...withLocalizePropTypes, ...toggleVisibilityViewPropTypes, }; const defaultProps = { account: {}, - isVisible: false, }; class PasswordForm extends React.Component { From 4da3cb1545bf688927e697df9dbed49dd81481bc Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sat, 4 Dec 2021 04:26:13 +0530 Subject: [PATCH 22/23] Reafactor code to remove isvisible dependency in PasswordForm UI and typos --- src/components/withToggleVisibilityView.js | 2 +- src/pages/signin/ChangeExpensifyLoginLink.js | 9 ++++++++- src/pages/signin/LoginForm.js | 3 +-- src/pages/signin/PasswordForm.js | 3 +-- src/pages/signin/SignInPage.js | 3 ++- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/components/withToggleVisibilityView.js b/src/components/withToggleVisibilityView.js index 42f9fc891293..3a98b09c205b 100644 --- a/src/components/withToggleVisibilityView.js +++ b/src/components/withToggleVisibilityView.js @@ -5,7 +5,7 @@ import styles from '../styles/styles'; import getComponentDisplayName from '../libs/getComponentDisplayName'; const toggleVisibilityViewPropTypes = { - /** Whether the content is viToggleVisibilitysible. */ + /** Whether the content is visible. */ isVisible: PropTypes.bool, }; diff --git a/src/pages/signin/ChangeExpensifyLoginLink.js b/src/pages/signin/ChangeExpensifyLoginLink.js index f600438ec401..9469828a21e6 100755 --- a/src/pages/signin/ChangeExpensifyLoginLink.js +++ b/src/pages/signin/ChangeExpensifyLoginLink.js @@ -16,11 +16,17 @@ const propTypes = { credentials: PropTypes.shape({ /** The email the user logged in with */ login: PropTypes.string, - }).isRequired, + }), ...withLocalizePropTypes, }; +const defaultProps = { + credentials: { + login: '', + }, +}; + const ChangeExpensifyLoginLink = props => ( @@ -45,6 +51,7 @@ const ChangeExpensifyLoginLink = props => ( ); ChangeExpensifyLoginLink.propTypes = propTypes; +ChangeExpensifyLoginLink.defaultProps = defaultProps; ChangeExpensifyLoginLink.displayName = 'ChangeExpensifyLoginLink'; export default compose( diff --git a/src/pages/signin/LoginForm.js b/src/pages/signin/LoginForm.js index 494215e63278..f70ccbb0a4de 100755 --- a/src/pages/signin/LoginForm.js +++ b/src/pages/signin/LoginForm.js @@ -50,7 +50,7 @@ class LoginForm extends React.Component { super(props); this.onTextInput = this.onTextInput.bind(this); this.validateAndSubmitForm = this.validateAndSubmitForm.bind(this); - this.clearLogin = this.clearLogin.bind(this); + this.state = { formError: false, login: '', @@ -65,7 +65,6 @@ class LoginForm extends React.Component { } componentDidUpdate(prevProps) { - // Whenever page becomes visible focus the input. We are using withToggleVisibilityView to hide the page instread of unmounting. if (prevProps.isVisible || !this.props.isVisible) { return; } diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js index 5bb3374f8d36..7b73a5937ee2 100755 --- a/src/pages/signin/PasswordForm.js +++ b/src/pages/signin/PasswordForm.js @@ -62,7 +62,6 @@ class PasswordForm extends React.Component { } componentDidUpdate(prevProps) { - // Whenever page becomes visible focus the input. We are using withToggleVisibilityView to hide the page instread of unmounting. if (prevProps.isVisible || !this.props.isVisible) { return; } @@ -159,7 +158,7 @@ class PasswordForm extends React.Component { isLoading={this.props.account.loading} onPress={this.validateAndSubmitForm} /> - {this.props.isVisible && } + ); diff --git a/src/pages/signin/SignInPage.js b/src/pages/signin/SignInPage.js index 1d50b31c5c35..538abbf79b2c 100644 --- a/src/pages/signin/SignInPage.js +++ b/src/pages/signin/SignInPage.js @@ -87,7 +87,8 @@ class SignInPage extends Component { shouldShowWelcomeText={showLoginForm || showPasswordForm || !showResendValidationLinkForm} shouldShowWelcomeScreenshot={showLoginForm} > - {/* Both LoginForm and PasswordForm should be present in the UI for Password Managers to work. */} + {/* 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. */} {showResendValidationLinkForm && } From d39fa49a8cae4411210bc60224412b0370a816aa Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Tue, 7 Dec 2021 01:52:00 +0530 Subject: [PATCH 23/23] fix prop definition --- .../ExpensiTextInput/ExpensiTextInputLabel/index.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js index b613c467a73e..b2c2abcff185 100644 --- a/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js +++ b/src/components/ExpensiTextInput/ExpensiTextInputLabel/index.native.js @@ -37,6 +37,6 @@ class ExpensiTextInputLabel extends PureComponent { } ExpensiTextInputLabel.propTypes = expensiTextInputLabelPropTypes.propTypes; -ExpensiTextInputLabel.propTypes = expensiTextInputLabelPropTypes.defaultProps; +ExpensiTextInputLabel.defaultProps = expensiTextInputLabelPropTypes.defaultProps; export default ExpensiTextInputLabel;