Skip to content
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

Make login form accessible to Password Managers #5275

Merged
merged 30 commits into from
Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5b66bdc
make Input accessible
parasharrajat Sep 15, 2021
8fcf1fd
fix: input for password Managers
parasharrajat Sep 15, 2021
228b1b2
adjust the Form for Password Managers
parasharrajat Sep 15, 2021
69f0e1d
Merge branch 'main' of github.com:Expensify/Expensify.cash into login…
parasharrajat Sep 15, 2021
31f6ebb
Merge latest chagnes from master
parasharrajat Nov 6, 2021
0c95acf
Refactor code
parasharrajat Nov 7, 2021
08a441e
Refactor Form
parasharrajat Nov 7, 2021
f5c1709
last touch up
parasharrajat Nov 7, 2021
28b22f1
Enable multi steo form
parasharrajat Nov 7, 2021
bf7859e
Multi step form support
parasharrajat Nov 7, 2021
b180d80
refactor page to set the focus correctly
parasharrajat Nov 8, 2021
62683de
Pull latest changes
parasharrajat Nov 8, 2021
e5b81eb
fix: Dom change
parasharrajat Nov 8, 2021
36ec46b
Pull latest changes
parasharrajat Nov 9, 2021
9f035f1
refactor
parasharrajat Nov 9, 2021
e621911
Pull new changes from main
parasharrajat Nov 24, 2021
14cb551
Merge branch 'main' of github.com:Expensify/Expensify.cash into login…
parasharrajat Dec 1, 2021
5938bd8
Review- 1
parasharrajat Dec 1, 2021
c12aeab
Merge branch 'main' of github.com:Expensify/Expensify.cash into login…
parasharrajat Dec 1, 2021
46430c7
Review comments
parasharrajat Dec 2, 2021
8b3127a
fix: Collapsible view
parasharrajat Dec 2, 2021
d7365f0
fix: form
parasharrajat Dec 2, 2021
9f5aa7b
pass down the name prop
parasharrajat Dec 2, 2021
9703fe6
typos
parasharrajat Dec 2, 2021
a1c5ac0
Clear form
parasharrajat Dec 2, 2021
530627f
Review -2
parasharrajat Dec 2, 2021
b594059
review-3
parasharrajat Dec 2, 2021
db6b4b6
Extra prop definition removed
parasharrajat Dec 2, 2021
4da3cb1
Reafactor code to remove isvisible dependency in PasswordForm UI and …
parasharrajat Dec 3, 2021
d39fa49
fix prop definition
parasharrajat Dec 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/components/ExpensiTextInput/BaseExpensiTextInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import ExpensiTextInputLabel from './ExpensiTextInputLabel';
import {propTypes, defaultProps} from './baseExpensiTextInputPropTypes';
import themeColors from '../../styles/themes/default';
import styles from '../../styles/styles';
import {setBrowserAttributes} from '../../libs/TextInputUtils';
import InlineErrorText from '../InlineErrorText';

const ACTIVE_LABEL_TRANSLATE_Y = -12;
Expand Down Expand Up @@ -42,10 +43,17 @@ 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.props.name) {
setBrowserAttributes(this.input, 'name', this.props.name);
}
}

componentDidUpdate(prevProps) {
Expand Down Expand Up @@ -150,6 +158,9 @@ class BaseExpensiTextInput extends Component {
innerRef,
autoFocus,
multiline,

// Only present for Web
name,
...inputProps
} = this.props;

Expand Down Expand Up @@ -184,6 +195,7 @@ class BaseExpensiTextInput extends Component {
}
labelTranslateY={this.state.labelTranslateY}
labelScale={this.state.labelScale}
for={this.props.nativeID}
/>
</>
) : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -13,6 +13,16 @@ const propTypes = {

/** Label scale */
labelScale: PropTypes.instanceOf(Animated.Value).isRequired,

/** For attribute for label (only Web) */
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
for: PropTypes.string,
};

const defaultProps = {
for: '',
};

export default propTypes;
export {
propTypes,
defaultProps,
};
63 changes: 38 additions & 25 deletions src/components/ExpensiTextInput/ExpensiTextInputLabel/index.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,44 @@
import React, {memo} from 'react';
import React, {PureComponent} from 'react';
import {Animated} from 'react-native';
import {setBrowserAttributes} from '../../../libs/TextInputUtils';
import styles from '../../../styles/styles';
import propTypes from './expensiTextInputLabelPropTypes';
import {propTypes, defaultProps} from './expensiTextInputLabelPropTypes';

const ExpensiTextInputLabel = ({
label,
labelTranslateY,
labelTranslateX,
labelScale,
}) => (
<Animated.Text
style={[
styles.expensiTextInputLabel,
styles.expensiTextInputLabelDesktop,
styles.expensiTextInputLabelTransformation(
labelTranslateY,
labelTranslateX,
labelScale,
),
]}
pointerEvents="none"
>
{label}
</Animated.Text>
);
class ExpensiTextInputLabel extends PureComponent {
constructor(props) {
super(props);
this.label = null;
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
}

componentDidMount() {
if (this.props.for) {
setBrowserAttributes(this.label, 'for', this.props.for);
}
}

render() {
return (
<Animated.Text
pointerEvents="none"
accessibilityRole="label"
ref={el => this.label = el}
style={[
styles.expensiTextInputLabel,
styles.expensiTextInputLabelDesktop,
styles.expensiTextInputLabelTransformation(
this.props.labelTranslateY,
this.props.labelTranslateX,
this.props.labelScale,
),
]}
>
{this.props.label}
</Animated.Text>
);
}
}

ExpensiTextInputLabel.propTypes = propTypes;
ExpensiTextInputLabel.displayName = 'ExpensiTextInputLabel';
ExpensiTextInputLabel.defaultProps = defaultProps;

export default memo(ExpensiTextInputLabel);
export default ExpensiTextInputLabel;
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -24,6 +24,7 @@ const ExpensiTextInputLabel = ({
);

ExpensiTextInputLabel.propTypes = propTypes;
ExpensiTextInputLabel.propTypes = defaultProps;
ExpensiTextInputLabel.displayName = 'ExpensiTextInputLabel';

export default memo(ExpensiTextInputLabel);
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ const propTypes = {
/** Input label */
label: PropTypes.string,

/** Name attribute for the input (only Web) */
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
name: PropTypes.string,

/** Input value */
value: PropTypes.string,

Expand Down Expand Up @@ -36,6 +39,7 @@ const propTypes = {

const defaultProps = {
label: '',
name: '',
errorText: '',
placeholder: '',
hasError: false,
Expand Down
25 changes: 25 additions & 0 deletions src/components/Form.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import React from 'react';
import {View} from 'react-native';
import {accessibilityRoleForm, setBrowserAttributes} from '../libs/TextInputUtils';

class Form extends React.Component {
componentDidMount() {
setBrowserAttributes(this.form, 'method', 'post');
setBrowserAttributes(this.form, 'action', '/');
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
}

render() {
return (
<View
accessibilityRole={accessibilityRoleForm}
accessibilityAutoComplete="on"
ref={el => this.form = el}
// eslint-disable-next-line react/jsx-props-no-spreading
{...this.props}
/>
);
}
}

Form.displayName = 'Form';
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
export default Form;
26 changes: 26 additions & 0 deletions src/libs/TextInputUtils/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* 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,
};
13 changes: 13 additions & 0 deletions src/libs/TextInputUtils/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const passwordAutocompleteType = 'password';
const accessibilityRoleForm = 'none';

/**
* Skip adding browser only attributes to underlying Nodes which will result in an runtime error.
*/
function setBrowserAttributes() {}

export {
passwordAutocompleteType,
accessibilityRoleForm,
setBrowserAttributes,
};
34 changes: 28 additions & 6 deletions src/pages/signin/LoginForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,23 @@ const propTypes = {
loading: PropTypes.bool,
}),

/** Whether the page is visible. */
visible: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe it is more common to see a boolean prop prefixed with is* so this could change to isVisible (but I'm sure there are inconsistencies there).


...windowDimensionsPropTypes,

...withLocalizePropTypes,
};

const defaultProps = {
account: {},
visible: 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);

Expand All @@ -54,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: ''});
}
}
}
marcaaron marked this conversation as resolved.
Show resolved Hide resolved

/**
* Handle text input and clear formError upon text change
*
Expand Down Expand Up @@ -98,19 +119,21 @@ class LoginForm extends React.Component {

render() {
return (
<>
<View style={!this.props.visible && styles.visuallyHidden}>
<View style={[styles.mt3]}>
<ExpensiTextInput
ref={el => this.input = el}
label={this.props.translate('loginForm.phoneOrEmail')}
value={this.state.login}
autoCompleteType="email"
autoCompleteType="username"
marcaaron marked this conversation as resolved.
Show resolved Hide resolved
textContentType="username"
nativeID="username"
name="username"
onChangeText={this.onTextInput}
onSubmitEditing={this.validateAndSubmitForm}
autoCapitalize="none"
autoCorrect={false}
keyboardType={getEmailKeyboardType()}
autoFocus={canFocusInputOnScreenFocus()}
translateX={-18}
/>
</View>
Expand Down Expand Up @@ -138,8 +161,7 @@ class LoginForm extends React.Component {
onPress={this.validateAndSubmitForm}
/>
</View>

</>
</View>
);
}
}
Expand Down
Loading